Page 1 of 1

Remove all simple forms of properties that are defined in table and simple form

Posted: Mon May 13, 2024 11:49 pm
by V0lcano
Problem:

mod compatibility is difficult when the icons for every entity have to be checked through either icons or icon and icon_size. It is even possible for icon_size to be defined but the icon to be defined through icons. When adding extra layers to the icons of other mods, this makes it mandatory to basically convert the other two properties into a table in icons. Reading other mods' icons requires looking at both icons and icon. The same goes for
  • results and result and result_count
  • position and positions in fluidboxes
  • dark_background_icon and dark_background_icons
  • icon_tintable and icon_tintables
  • icon_tintable_mask and icon_tintable_masks
  • icon_color_indicator_mask and icon_color_indicator_masks
  • layers and everything else in AnimationPrototype
  • graphics set and animation and base_picture
Solution:
remove the simple definition and force mods to use the table definition.
So instead of icon = "__mod__/icon.png" it would be icons = {{icon = "__mod__/icon.png"}}
and instead of result = "transport belt" it would be results = {{result="transport-belt", amount = 1}}
for simple definitions

Re: Remove all simple forms of properties that are defined in table and simple form

Posted: Tue May 14, 2024 12:02 am
by BrainGamer_
General +1 to removing duplicate definition possibilities for things like this.

One addition I'd like to make is that the special cases where the icon property of IconData gets renamed to the singular names of dark_background_icons, icon_tintables, icon_tintable_masks and icon_color_indicator_masks should be removed and just also use the normal icon property instead.

Re: Remove all simple forms of properties that are defined in table and simple form

Posted: Tue May 14, 2024 12:28 am
by commanderguy3001
I do heavily support this change.
There are however many more instances of this in the API.

A (hopefully mostly complete) list of things that should be kept in favor of their direct assignment counterparts:

For Prototypes:
- the entirety of AnimationPrototype
- BeaconPrototype.graphics_set
- ElectricEnergyInterfacePrototype.pictures
- EntityPrototype.icons
- ItemPrototype.dark_background_icons
- ItemWithEntityDataPrototype.icon_tintable_masks and icon_tintables
- MiningDrillPrototype.graphics_set
- RecipePrototype.icons
- RecipePrototype.results
- SoundPrototype.variations
- TilePrototype.icons
- rocket_launch_products for everything that inherits from ItemPrototype

For Types:
- AnimationVariations.sheets
- BaseModifier.icons
- BurnerEnergySource.fuel_categories
- MinableProperties.results
- PipeConnectionDefinition.positions
- RecipeData.results
- Sound.variations
- Sprite4Way.sheets
- Sprite8Way.sheets
- WorkingVisualisation.animation


Removing the raw assignment for all of these and instead replacing them with arrays of length one would be a relatively simple change on the lua side, but would significantly cut the code required for mods to dynamically interacting with any of the named proto-/ types by an order of magnitude.

Furthermore, I expect that you can probably remove a few thousand lines of c++ code for handling the different variations as well. (which I guess would technically also speed up the engine data.raw parsing part of the entire prototype stage)

Re: Remove all simple forms of properties that are defined in table and simple form

Posted: Tue May 14, 2024 2:40 am
by Rseding91
As a general answer: "no"

Some things are changed in 2.0 but most of what you listed are not going to be changing.
commanderguy3001 wrote:
Tue May 14, 2024 12:28 am
Furthermore, I expect that you can probably remove a few thousand lines of c++ code for handling the different variations as well. (which I guess would technically also speed up the engine data.raw parsing part of the entire prototype stage)
Supporting loading from variants like this is trivial, on average 3 lines per class that does it. Most classes are re-used (all icon loading is 1 function with 3 lines to support icon vs icons). Sprite sheets is also 3 lines and re-used anywhere you see "sheets."

Re: Remove all simple forms of properties that are defined in table and simple form

Posted: Tue May 14, 2024 8:06 am
by curiosity
Rseding91 wrote:
Tue May 14, 2024 2:40 am
Some things are changed in 2.0 but most of what you listed are not going to be changing.
That's a shame.
V0lcano wrote:
Mon May 13, 2024 11:49 pm
  • layers and everything else in AnimationPrototype
You do realize that layers are recursive, right? When do you stop then, if it's always layers?

Re: Remove all simple forms of properties that are defined in table and simple form

Posted: Tue May 14, 2024 1:10 pm
by BrainGamer_
Rseding91 wrote:
Tue May 14, 2024 2:40 am
Some things are changed in 2.0
are the icon / IconData definitions changed? because they do have quite some weird / unexpected interactions between renaming fields of IconData depending on where its used, requiring some field defined outside of IconData if some member has a field missing, ...

Re: Remove all simple forms of properties that are defined in table and simple form

Posted: Tue May 14, 2024 7:00 pm
by pleegwat
A less breaking version of this would be to normalize in data:extend(), so you can initially set either version but when reading you always get the array version. That would probably be more implementation work though.

Re: Remove all simple forms of properties that are defined in table and simple form

Posted: Wed May 15, 2024 8:47 am
by curiosity
pleegwat wrote:
Tue May 14, 2024 7:00 pm
A less breaking version of this would be to normalize in data:extend(), so you can initially set either version but when reading you always get the array version. That would probably be more implementation work though.
You could do that with a mod. But also dealing with direct assignment will be complicated.

Re: Remove all simple forms of properties that are defined in table and simple form

Posted: Wed May 15, 2024 12:58 pm
by Nidan
curiosity wrote:
Wed May 15, 2024 8:47 am
pleegwat wrote:
Tue May 14, 2024 7:00 pm
A less breaking version of this would be to normalize in data:extend(), so you can initially set either version but when reading you always get the array version. That would probably be more implementation work though.
You could do that with a mod. But also dealing with direct assignment will be complicated.
The tracking of which mod changed which prototype seems to be very reliable, so that could be used to figure out what may need normalization. But instead of each data:extend I'd do it after each individual data/data-updates/data-final-fixes.lua is done.

While a mod could walk through data.raw and normalize it, it can only to so once per data*.lua and other mods could interfere, so it isn't reliable for anyone but the mod itself.

Re: Remove all simple forms of properties that are defined in table and simple form

Posted: Wed May 15, 2024 1:15 pm
by curiosity
Nidan wrote:
Wed May 15, 2024 12:58 pm
The tracking of which mod changed which prototype seems to be very reliable, so that could be used to figure out what may need normalization. But instead of each data:extend I'd do it after each individual data/data-updates/data-final-fixes.lua is done.
The problem is not that the game has to handle those abnormal definitions, but all other mods.

Re: Remove all simple forms of properties that are defined in table and simple form

Posted: Wed May 15, 2024 7:40 pm
by Nidan
curiosity wrote:
Wed May 15, 2024 1:15 pm
Nidan wrote:
Wed May 15, 2024 12:58 pm
The tracking of which mod changed which prototype seems to be very reliable, so that could be used to figure out what may need normalization. But instead of each data:extend I'd do it after each individual data/data-updates/data-final-fixes.lua is done.
The problem is not that the game has to handle those abnormal definitions, but all other mods.
I know. (maybe you misunderstood?) My suggestion was to normalize after every single data*.lua. E.g. mod-a/data.lua; normalize; mod-b/data.lua; normalize; mod-a/data-updates.lua; normalize; mod-b/data-updates; normalize; mod-a/data-final-fixes.lua; normalize; mod-b/data-final-fixes.lua.
While each mod must deal with its own mess, before another mod can see it (except requires into other mods' code), factorio would've transformed it into the standard form. And I'm under the impression that such a normalization would be relatively cheap since factorio can already track in which order which mods modified which prototype.

Re: Remove all simple forms of properties that are defined in table and simple form

Posted: Thu May 16, 2024 12:34 pm
by KiwiHawk
Rseding91 wrote:
Tue May 14, 2024 2:40 am
As a general answer: "no"

Supporting loading from variants like this is trivial, on average 3 lines per class that does it. Most classes are re-used (all icon loading is 1 function with 3 lines to support icon vs icons). Sprite sheets is also 3 lines and re-used anywhere you see "sheets."
The reason for the request isn't to make things easier for the core game. It's to make things easier (and less error prone) for other mods!

We would very much appreciate if the general answer could be reconsidered! Please? 🙂