Enhance OnSet and OnRemove hooks to include previous value

[chess123mate]

Feature

OnSet and OnRemove would both provide the previous value, becoming: OnSet(entity, value, prevValue) and OnRemove(entity, prevValue).

Motivation

I'm having to regularly create maps of entityToValue for various components so that I can perform cleanup specific to the component in OnSet and OnRemove (such as disabling or destroying a Roblox Instance referenced by the component, or for knowing which value to update in a valueToEntity map), and it feels wasteful when JECS is obviously already tracking entityToValue.

This would also fix the use-case described here:

This happens because the destination of where the entity should move is calculated after. So the order of the operations should be swapped. It will break existing behaviour where people rely on getting the data on remove.

Originally posted by @Ukendio in https://github.com/Ukendio/jecs/issues/118#issuecomment-2355892792

Considerations

• Will old code break in response to this feature? No
• What are the performance impacts with this feature (if any)? Very minimal for cases that don't use this feature; will be faster than the using code tracking entityToValue separately

[Ukendio]

I see your concern, and I suggest you keep state in pair(Previous, component) pairs instead. Hooks are not the intended way to provide an interface for evaluating assignments of data.

There are several other ECS that does similar things, e.g. in hecs you would use a Previous<T> struct that would newtype the component. Such as query<(T, Previous<T>). And in flecs you would simply have a relationship similar to what aforementioned, e.set<Component, Previous>({ ... });.

OnRemove already provides a stable reference to the entity before it gets cleaned up.

world:set(Model, OnRemove, function(entity) 
    world:get(entity, Model):Destroy()
end)

[chess123mate]

You mentioned elsewhere that Hooks are a means of listening to constructing/deconstructing the object. But based on what you've said, they currently don't serve that purpose:

• You can use OnRemove for destructuring as you've shown
• Currently you can use OnSet for constructing, but you can't use it for deconstructing (without storing both entityToValue and valueToEntity). You seem to suggest not using OnSet at all, but rather track it via a component? If you're supposed to use a component, what's the point of the hooks again? I mean, if you're supposed to have a separate system that iterates over all "Model" without "Previous" for construction and then over all "Previous" for changes & removals, where do hooks fit in?

Further, just to be sure: wouldn't query<(T, Previous<T>)> be unable to differentiate between "changed" and "unchanged"? You'd have to iterate over every component every update, just to see if a change has occurred. (This seems less performant than just OnSet passing in the previous value.)

Now, I don't know what other use-cases the hooks are meant for, but if it's "not for changing entities" (and if you choose not to go for what I'm proposing in #118 ), might it be better to have "OnSet" be used for construction and "OnRemove" for destruction (meaning that when you set a new value, you first call OnRemove, then you update the value, then you call OnSet)? If construction/destruction is the primary use-case, this would simplify the API.

(Of course, if destruction is the only reason to use OnRemove, then I don't know why you wouldn't just pass the value anyway, as that's faster than world:get(entity, Model).)

[Ukendio]

• Currently you can use OnSet for constructing, but you can't use it for deconstructing (without storing both entityToValue and valueToEntity). You seem to suggest not using OnSet at all, but rather track it via a component? If you're supposed to use a component, what's the point of the hooks again? I mean, if you're supposed to have a separate system that iterates over all "Model" without "Previous" for construction and then over all "Previous" for changes & removals, where do hooks fit in?

This really depends on what data you are working with. Hooks are definitely useful for deconstruction of data because it provides you a stable point of access to the entity which you can generically apply behaviour specific to the hooked component. Previous pairs won't work during deletion of an entity because deletion removes all components at once which means it won't be picked up in queries anymore.

Further, just to be sure: wouldn't query<(T, Previous<T>)> be unable to differentiate between "changed" and "unchanged"? You'd have to iterate over every component every update, just to see if a change has occurred. (This seems less performant than just OnSet passing in the previous value.)

For direct mutations and delta comparisons of data, it would be more beneficial to use (Previous, T) pairs as you can directly compare the data of entities with your own logic and process them in bulk. This is more efficient as you are linearly accessing entities in a contiguous manner as opposed to listening to changes that follow multiple callbacks (set -> invoke_hook -> fn).

Now, I don't know what other use-cases the hooks are meant for, but if it's "not for changing entities" (and if you choose not to go for what I'm proposing in #118 ), might it be better to have "OnSet" be used for construction and "OnRemove" for destruction (meaning that when you set a new value, you first call OnRemove, then you update the value, then you call OnSet)? If construction/destruction is the primary use-case, this would simplify the API.

If you don't have direct mutations and changes are far in-between for that component on the entity, then hooks would incur less overhead over time. Hooks are kind of like virtual methods you can override on a component. E.g. you may want to set a WorldTransform after LocalTransform has changed, the old data is completely irrelevant here.

(Of course, if destruction is the only reason to use OnRemove, then I don't know why you wouldn't just pass the value anyway, as that's faster than world:get(entity, Model).)

The hooked component might be a tag or some component whose value is irrelevant but is indicative that you need to handle some other reference to a thing in the datamodel. Incurring the cost of a random access is unnecessary there. So in the best case passing the value is more ergonomic without performance benefit, and in the worst case do unnecessary computation