Ukendio / jecs

A fast, portable Entity Component System for Luau
https://ukendio.github.io/jecs/
MIT License
146 stars 26 forks source link

[BUG] Potential bug (or lack of documentation): hook triggers (OnAdd, OnSet, OnRemove) are inconsistent #119

Closed chess123mate closed 1 month ago

chess123mate commented 2 months ago

Description

Repeated calls to world.set may trigger OnSet even when the value doesn't change (such as if you were to call world.set(entity, speed, 10) multiple times in a row), but repeated calls to world.remove only triggers OnRemove once.

Note that component-traits.md indicates for OnSet -- A transform component has been assigned/changed to value on the entity, but the word changed is misleading.

Expected Behavior

Any of:

  1. Don't trigger OnSet if the current value is the same as the new value (I'd think this is the intuitive option)
  2. Clarify the existing behaviour in the documentation (at least in component-traits.md, but optionally also in jecs/src/index.d.ts)
  3. Make world.add and world.remove always trigger OnAdd or OnRemove, even if the value is already added/removed (this would make the behaviour consistent with OnSet, though I doubt this option is desirable).

I noticed this issue because I have a system that regularly transforms the values from one component into the value of another - so, when the input component is unchanged, the output is also unchanged. I'm relatively new to ECS, so maybe there's an elegant solution (it occurs to me that one could add/remove a component "flag" indicating whether the calculation needs to be done, and then modify the world.query appropriately), but it seems to me that if the formula isn't too complex, it'll be faster (or at least easier) to just blindly run the transformation code. I would think this is a relatively common pattern in ECS? If so, I'd think option 1 might be best - unless you're trying to do everything possible for maximum speed, in which case I'd suggest improved documentation.

Ukendio commented 1 month ago

This is the expected behaviour because some people use tables as values and when they write direct mutations like that they usually want to propagate the change manually.