PylonItemStackBuilder

[JustAHuman-xD]

A specific variant of the ItemStackBuilder with util methods for common item settings & components, also adds some more methods to the normal ItemStackBuilder.

TODO:

• KDocs

[Intybyte]

ToolItemStackBuilder would be better, no reason to call it Pylon

[JustAHuman-xD]

It does way more than just tools, requires a pylon item key, and uses pylon item settings, why would it not be pylon?

[LordIdra]

Tbh this does feel more like it should be either part of the ItemStackBuilder, or possibly a set of utility methods that create the components. I can see there potentially being a lot of confusion over 'why are there 2 item stack builders' and 'why is the PylonItemStackBuilder not needed to create pylon items'. Is there any reason these can't just be methods on the original ItemStackBuilder?

[JustAHuman-xD]

I initially did that but then thought it may bring confusion, if someone is making an item stack and wants to give it a weapon, or custom durability or something, and doesn't read the docs, it would throw an exception because there are no settings for a non pylon item.

I included the overloads for people that don't want to have values configurable but I suppose it also would allow for non-pylon items to use the methods assuming they provide a value for every field and don't use any defaults.

I'd also then have to add a field to ItemStackBuilder for a pylon item key. Which isn't necessarily good or bad it's just a change.

[Seggan]

What if we move all the Pylon-specific stuff (like pylonItem) into PylonItemStackBuilder and have the normal one be just for vanilla/ui?

[LordIdra]

Another thing maybe worth discussing: Are we sure we want to have Settings.get in these methods as a default? To me that feels like a bit of an overreach, as it's hiding away behaviour that might be better made explicit rather than put behind documentation. I think it might be best to keep settings keys determined exclusively by addons rather than mixing it.

[JustAHuman-xD]

Another thing maybe worth discussing: Are we sure we want to have Settings.get in these methods as a default? To me that feels like a bit of an overreach, as it's hiding away behaviour that might be better made explicit rather than put behind documentation. I think it might be best to keep settings keys determined exclusively by addons rather than mixing it.

Thats the main point behind this though, if we don't have settings.get as defaults, if someone wants to make an equipment set thats configurable, they have to manually set that out, for every piece, which is just a TON of copying and pasting, if you want to make multiple gear sets? Now its even more. I was/am going to write documentation that explains what values you have to have for them, and in the docs site ill add examples, but the point behind it is to save on those cases. If we don't have this in core I will honestly probably for my own sanity, make a variant that adds them because I dont want to be copying and pasting those settings values every single time.

[LordIdra]

Is the copy/pasting really that much of an issue? It's not like we're making 10,000,000 armour sets here, and it's not like you have to change much after the item has been created. And if you are then you can create a utility function in your addon.

My main concern is we shouldn't be adding extra/unexpected behaviour for the sake of small ergonomics and saying 'oh we'll just document it' for cases like this. Core needs to be kept slim, simple, and direct to make it appealing and simple to develop addons for.

Proposal: Move this to the ItemStackBuilder and remove the Settings.get for the armour, armour-toughness. We already use Settings.get in PylonItems in base for most items. That way no assumptions are made and no docs are needed. It's only a little bit of extra code and it's very obvious what is going on.

[LordIdra]

we argued for like 20 mins about this and got absolutely nowhere so requesting third opinions ^^^^^

[Seggan]

My personal opinion leans towards @LordIdra's. However, I would not be against creating a utility function for this.

Now that I look at it, moving all the Pylon-specific stuff (name and lore stuff mostly) to a PylonItemStackBuilder I wouldn't be against it.

[LordIdra]

I think if we move all the pylon stuff to PylonItemStackBuilder it should be fine. The main contention is whether we should have the default Settings.get calls

[Seggan]

Yeah no, maybe a utility function for this but not as an integral part.

[LordIdra]

Yeah no, maybe a utility function for this but not as an integral part.

We can't really have a utility function because you'd have to basically pass the whole builder into the utility function. It does lots of different things on the stack builder

[LordIdra]

Yeah no, maybe a utility function for this but not as an integral part.

We can't really have a utility function because you'd have to basically pass the whole builder into the utility function. It does lots of different things on the stack builder

it's all or nothing basically

[Seggan]

Why not just a Utils.createHelmet(), Utils.createChestplate(), etc?

[LordIdra]

Why not just a Utils.createHelmet(), Utils.createChestplate(), etc?

I would suggest PylonItemStackBuilder.helmet() instead. I would still prefer that no config values be retrieved as default but this is a compromise that I think is fine

[Seggan]

Yeah idc where exactly it is, the Utils was just an example