-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Object properties remove colors
#15685
base: master
Are you sure you want to change the base?
Conversation
d5b2a0e
to
8dee4d9
Compare
I think hardware-coloring for entities isn't so far-fetched so I'd keep this property but just document clearly that it does nothing right now. |
As a minor side note, removing this property is technically a breaking change: Mods could be relying on This seems unlikely to me since this doesn't do anything right now, but you never know. Maybe someone is abusing this to store a couple colors (which is a really odd, bad way to store that, but 🤷). |
Is this really something we want to add? The documentation for object properties says:
So I think it is clear enough that you can't use those to store your own data. There could always be someone abusing unused things, e.g. if you don't use a |
Maybe. The problem with how texture modifiers work is that it requires creating and processing a new texture. Hardware coloring could be more efficient since all you have to do is pass a bunch of colors to a shader. Arguably we could probably implement hardware coloring as a special optimization for texture modifiers here, so something like "if it's simply some base texture with a
Well, it seems to me that the way the docs are written, and the way the code behaves, we are guaranteeing that this property exists, that you can put colors in there and get them back out, same as for the other properties, while the entity is active (whether the entity is persisted or not; "Store" doesn't mean "store persistently" here). Modders shouldn't be doing that, since for entities using a custom field, say Though as said, this is in my opinion very minor. I wasn't sure whether I should've brought it up at all. |
This PR removes the
colors
field from object properties.It is not (sufficiently) documented, quite useless and takes up space.
This is the only place where it gets used, so only for
upright_sprite
withglow < 0
, and the default color is white,but I didn't notice any difference, so I completely removed it instead of defaulting it to white., readded it, but I'm not sure if is needed.https://github.com/minetest/minetest/blob/c8b5e3b741390fb98bfa9cdbae56930239ea271a/src/client/content_cao.cpp#L1406-L1408
The documentation for
glow
saysValue < 0 disables light's effect on texture color.
and this sill works.To do
This PR is Ready for Review.
How to test