Skip to content

Conversation

@JustAHuman-xD
Copy link
Contributor

@JustAHuman-xD JustAHuman-xD commented Sep 30, 2025

Supercedes #325
Based on #341 as it modified the stack builder already and started on some of the changes I needed for this

Adds a PylonItemStackBuilder with new utils and moves the previous pylon item related ones from ItemStackBuilder to it.

Adds a number of new utility methods for both.

Gives ui items specific keys to be identifiable, also gives PylonFluid items a specific key and adds a method to get the fluid from the item stack representation

@JustAHuman-xD
Copy link
Contributor Author

Marked as a draft on the chance I misunderstood the resolution we came to in #325 but reviews are welcome even now

@LordIdra
Copy link
Contributor

LordIdra commented Oct 3, 2025

Have thought about this for a while and I don't like the fact that PylonItemStackBuilder is named as such but is not actually a builder (or even a class), but a set of utility methods that return builders. It feels quite misleading. It's also rather unclear at first glance why for example there is an armor method on both ItemStackBuilder and PylonItemStackBuilder, and which one you should use. I would propose that we remove PylonItemStackBuilder, migrate its methods and prefix them with pylon. @Seggan what are your thoughts

@Seggan
Copy link
Member

Seggan commented Oct 3, 2025

Sure

Base automatically changed from use-vanilla-entities-in-pylon-entity-block-holder to master October 4, 2025 16:37
@OhmV-IR
Copy link
Contributor

OhmV-IR commented Oct 4, 2025

doesn't compile, no review for you

@JustAHuman-xD
Copy link
Contributor Author

JustAHuman-xD commented Oct 4, 2025

#blame-idra he broke master, you can still review it though?? (/lh)

@JustAHuman-xD JustAHuman-xD requested a review from OhmV-IR October 4, 2025 23:31
Copy link
Contributor

@OhmV-IR OhmV-IR left a comment

Choose a reason for hiding this comment

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

cw stuff

@JustAHuman-xD JustAHuman-xD linked an issue Oct 7, 2025 that may be closed by this pull request
Copy link
Contributor

@LordIdra LordIdra left a comment

Choose a reason for hiding this comment

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

Happy with this (when ohm's comments addressed). Not sure about pylonItem being renamed to pylon but don't care enough to block

@LordIdra LordIdra merged commit 87d4210 into master Oct 13, 2025
3 checks passed
@LordIdra LordIdra deleted the update-item-builders branch October 13, 2025 02:21
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.

Add keys to GUI items so they can be textured

4 participants