Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IC2: Add converter for reactor components to get stored heat. #126

Merged

Conversation

Engineer12798
Copy link

With coolant cells and other heat storing items, there is no uniform way in game to extract the heat stored in these items. To do so, would require parsing the NBT of the item stack, which is difficult enough as is, but would also require reading different NBT tags depending on the mod the item came from. To resolve this problem, this PR adds in a converter which will get the stored heat of any item implementing IReactorComponent.

The way the PR works may be jank and not work with some components. There may exist cases where the stored heat and max heat of a component are dependent on the reactor context, which this converter does not provide when trying to get the stored heat.

@Dream-Master Dream-Master requested a review from a team July 9, 2024 13:29
@OneEyeMaker
Copy link

OneEyeMaker commented Jul 9, 2024

Despite this implementation will work in certain circumstances, one doesn't solve mentioned issue properly. (Even more, one can cause an exception).

Instead I suggest to extend DriverReactor and DriverReactorChamber classes and add necessary methods there.

And only with beforementioned changes present I can suggest to add your implementation for handling components outside reactor. Make sure to mark it explicit and unsafe so players understand what they do. Currently, from player's perspective, these properties you added look like regular (safe) ones.

Copy link

@OneEyeMaker OneEyeMaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job! Now this feature is almost complete.
Just find a way to tell players that original approach is not 100% reliable.

@Dream-Master Dream-Master requested a review from OneEyeMaker July 11, 2024 00:41
@Dream-Master Dream-Master merged commit e3fa228 into GTNewHorizons:master Jul 11, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants