Skip to content
New issue

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

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

Already on GitHub? # to your account

Check Fuel as an ItemStack #19

Merged
merged 4 commits into from
Mar 26, 2021

Conversation

serenibyss
Copy link

@serenibyss serenibyss commented Mar 19, 2021

The remaining fix from #17 not included in the config-refactor branch was switching to using ItemStacks for fuel comparison. With this change, NBT data and metadata is properly respected for cake fuels.

For example, the Overworld Cake is set to oakSapling by default for its fuel. However, it would previously accept any Vanilla Sapling. Now it is explicitly Oak.

I left a method BlockCakeBase#getFuelItemStack as separate from the one place currently calling it, BlockCakeBase#onBlockActivated because I use this method again in the fuel tooltip.

@serenibyss serenibyss mentioned this pull request Mar 19, 2021
@ALongStringOfNumbers
Copy link
Member

Is there a point to having the String cakeFuel method in the base class and the overrides in the other cakes anymore if the switch to ItemStacks are made?

@serenibyss
Copy link
Author

serenibyss commented Mar 21, 2021

All of that was removed in the config-refactor branch, which is why I am requesting to merge into that branch instead of into master, unless there is something else I am missing that can be removed.

@ALongStringOfNumbers
Copy link
Member

ah, my bad. I misread the target branch

@Exaxxion Exaxxion merged commit 676b996 into Nomifactory:config-refactor Mar 26, 2021
Exaxxion added a commit that referenced this pull request Apr 9, 2021
* Switch to ItemStack for fuel
* Add a fallback value for default cake fuels in case of config error
* Use default behavior for getFuelItemStack

Co-authored-by: Exa <11907282+Exaxxion@users.noreply.github.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants