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

Provide single-property alternatives to [gs]et_properties #15675

Open
appgurueu opened this issue Jan 13, 2025 · 3 comments
Open

Provide single-property alternatives to [gs]et_properties #15675

appgurueu opened this issue Jan 13, 2025 · 3 comments
Labels
Feature request Issues that request the addition or enhancement of a feature Performance @ Script API

Comments

@appgurueu
Copy link
Contributor

Logical extension of #14418 to the Lua sphere of things.

set_properties and get_properties are currently needlessly inefficient: The former will check for all possible properties. The latter will push all properties to a table. This is very wasteful if you just want to get or set a single property.

The obvious solution is to provide single-property getters and setters.

Let's call them obj:set_property(name, value) and obj:get_property(name) respectively.

The implementation for these should probably be via an unordered map from property names to getter-setter function pointer pairs. (Or a variation of this, for example this could also alternatively use member references and property types to auto-generate these pairs at compile time.)

This could also be used to optimize the common use case of set_properties where you only provide a table with a few fields by iterating the table and looking up & only running corresponding setters.

@appgurueu appgurueu added @ Script API Feature request Issues that request the addition or enhancement of a feature Performance labels Jan 13, 2025
@rubenwardy
Copy link
Contributor

A nice API would be to use metatables to allow modders to do self.object.properties.name = value

@Desour
Copy link
Contributor

Desour commented Jan 14, 2025

A nice API would be to use metatables to allow modders to do self.object.properties.name = value

No, please don't hide what is happening. Magic just makes thing more difficult to understand.

@appgurueu
Copy link
Contributor Author

appgurueu commented Jan 14, 2025

In the end it doesn't matter much. What matters is that there is some way to only get or set single properties under the hood.

Implementing a metatable-based API would be trickier and also, as Desour said, make it less obvious what's going on.

Modders can easily add magic themselves if they want or need to. For example:

core.register_entity("...", {
  properties = function(self)
    return setmetatable({}, {__index = function(_, k) return self.object:get_property(k) end, __newindex = function(_, k, v) return self.object:set_property(k, v) end})
  end,
  ...
})

And then they have to deal with the warts of this (for example no pairs or next support, see also #15133) themselves.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature request Issues that request the addition or enhancement of a feature Performance @ Script API
Projects
None yet
Development

No branches or pull requests

3 participants