EventResult handler(Event event, long entityId, Transaction transaction [, ComponentA a, ComponentB b...)
Gratulations then for reaching this milestone.I have been making some progress on the event system,which is visible in my fork of gestalt
Do you plan on adding something that prevents big entities to being copied unnecessary each time you look at them? Also what is with entities with a lot of data, that have very frequently some of it's small components modified. e.g. a character might have a ton of components from different modules that add their own character specific static meta data(e.g. descriptions of the armor, quests the player has in progress, etc.) but has on the other side a lot of frequently updated data like (location, mana bar etc).First thing of note is that core event processing makes full use of transactions. The transaction an event is running in acts as a cache of the state of the target entity, providing all the components of for the event handlers. I tweaked how transactions worked a touch in the current reference implementation to go with this - whenever an entity is touched all of its components are pulled into the transaction. This implies that later when I the entity system is being optimised that obtaining all the components of an entity at once is an important operation to focus on. It fits well with the revision number tracking being at the entity level too.
That sounds like you remove an optimization. So I am curious what your reasoning where.Beyond that, the core event processing has been cleaned up a lot. In terasology the event system tries to filter out event handlers at the beginning of processing an event, based on available components. This has been thrown away and the checking is now done for each event handler as the processor works through the list.
Sounds reasonable.Pulling events handler methods out of an object has been moved into a helper class separate from the core processing class too. Networking stuff is all removed - this would be added at a higher level as desired.
With "class providing an event." I guess you mean the system that defines a handler for it. Sounds good, was long overdue.Event handler priority has also been removed, in favor of explicit @Before and @After annotations that allow ordering before or after the class providing an event.
Having the option to still use synchronous events is good, as I guess most of the existing code relies on that.Moving out of core processing, events can now either be Synchronous or Asynchronous (with Asynchronous being the default). Synchronous events are run immediately and within the transaction sending them (if any). Asynchronous events are only run after the transaction sending them has been successfully committed, and can run at any time depending on how the EventSystem running them is implemented - the only guarantee is when EventSystem:rocessEvents() is called it will block until all outstanding events have been processed (and any events generated processing them). Two implementations are currently done - an EventSystem that runs all events immediately, and an EventSystem that queues up Asynchronous events until processEvents() is called.
For Asynchronous events, if the transaction fails to commit at the end due to a ConcurrentModificationException (conflict with another, completed transaction) then the event will automatically be re-run.
entityId? What happened to EntityRef? Maybe it should be allowed to still formulate event handlers in the old format and have the old format default to synchronous event handling.The event receiver signature is also a little different:
Code:EventResult handler(Event event, long entityId, Transaction transaction [, ComponentA a, ComponentB b...)
I like the idea of having a return value instead of a consumable event.EventResult is an enum which is either CONTINUE, COMPLETE or CANCEL. Continue tells the event processor to continue with the next handler. Complete tells the event processor to stop calling handlers and treat the event as successful. Cancel tells the event processor to stop calling handlers and treat the event as failed - the transaction will rolled back.
That is a good question. I'm leaving optimisation to the side for the moment to focus on the desired behavior, but this probably should be set to copy the component when the component is first requested, not when the entity is pulled into the transaction.Do you plan on adding something that prevents big entities to being copied unnecessary each time you look at them? Also what is with entities with a lot of data, that have very frequently some of it's small components modified. e.g. a character might have a ton of components from different modules that add their own character specific static meta data(e.g. descriptions of the armor, quests the player has in progress, etc.) but has on the other side a lot of frequently updated data like (location, mana bar etc).
Partially correctness of behavior, partially an actual optimisation. The incorrectness of behavior was around what would happen if an earlier handler added a component that would make a later handler valid - in terasology because the filtering occurs before processing the chain the later handler would already be removed. The optimisation is because the correct components were being checked again before each call anyway. Just taking the entire chain and then checking each handler as it is reached is simpler and more correct.That sounds like you remove an optimization. So I am curious what your reasoning where.
In general, yes - it would be the component system class in Terasology.With "class providing an event." I guess you mean the system that defines a handler for it. Sounds good, was long overdue.
It is necessary for query-style events at the very least.Having the option to still use synchronous events is good, as I guess most of the existing code relies on that.
Currently EntityRef is not a thing in the new entity system. Even if it was, it would lose all its methods, pretty much - I don't have a good answer for how the methods would work in the presence of transactions and multiple threads. Also I don't want to eat the potential memory usage and churn of essentially wrapping a long in a class without thinking it through. I may well end up reintroducing it, we'll see how things turn out.entityId? What happened to EntityRef? Maybe it should be allowed to still formulate event handlers in the old format and have the old format default to synchronous event handling.
In a lot of cases components are only obtained for reading data. That might be also a point where a optimization could be added.That is a good question. I'm leaving optimisation to the side for the moment to focus on the desired behavior, but this probably should be set to copy the component when the component is first requested, not when the entity is pulled into the transaction.
Hmm, seems to me like a corner case that doesn't make any difference in common scenarios. While on the other hand I thought the optimization to filter out relevant systems at start is very imporant for generic events like OnChangedComponent which will propably have a ton of handlers and a lot of calls.Partially correctness of behavior, partially an actual optimisation. The incorrectness of behavior was around what would happen if an earlier handler added a component that would make a later handler valid - in terasology because the filtering occurs before processing the chain the later handler would already be removed. The optimisation is because the correct components were being checked again before each call anyway. Just taking the entire chain and then checking each handler as it is reached is simpler and more correct.
hmm, I don't really see the man power in terasology (which I mainly mean motivated people to do just boring porting work) to do all the necessary adjustments to adjust to a big API change. I for example have a student to mentor and want to get some content (and thus example code) done which I think is currently more important for terasology.Currently EntityRef is not a thing in the new entity system. Even if it was, it would lose all its methods, pretty much - I don't have a good answer for how the methods would work in the presence of transactions and multiple threads. Also I don't want to eat the potential memory usage and churn of essentially wrapping a long in a class without thinking it through. I may well end up reintroducing it, we'll see how things turn out.
What actions do you want do with transactions?Other than that, I think the Transaction is important enough to include, otherwise there is no way to do a lot of actions.
True.In a lot of cases components are only obtained for reading data. That might be also a point where a optimization could be added.
Events with triggering components is probably the once space where this could help. But... the structure used by terasology to make it efficient to do the initial filtering loses order, so the handlers have to be sorted afterwards. Which is also a cost. Any way, without metrics we're just arguing about airy supposition which is pointless.Hmm, seems to me like a corner case that doesn't make any difference in common scenarios. While on the other hand I thought the optimization to filter out relevant systems at start is very imporant for generic events like OnChangedComponent which will propably have a ton of handlers and a lot of calls.
To make things very clear, I don't care whether Terasology ends up using gestalt-entity-system or not. If the design is too much work for Terasology to integrate, don't. It isn't useful for Terasology in its current state anyway.hmm, I don't really see the man power in terasology (which I mainly mean motivated people to do just boring porting work) to do all the necessary adjustments to adjust to a big API change. I for example have a student to mentor and want to get some content (and thus example code) done which I think is currently more important for terasology.
If we really want event transactions with multi threading for terasology then we could put the transaction in a thread local variable (https://docs.oracle.com/javase/8/docs/api/java/lang/ThreadLocal.html) and then access it from the method of EntityRef.
Without a transaction you cannot addComponents, removeComponents or send events. Or work with other entities.What actions do you want do with transactions?
Maybe just a "auto save components at end of event processing" feature would be enough for terasology.
{
"inherit" : "core:parent",
"root" : "person",
"entities" : {
"person": {
"player": {
"name": "Fred"
},
"inventory" : {
"items" : [
"core:torch", "core:torch", "extension:axe", "journal"
]
}
},
"journal": {
"book": {
"cover": "Journal"
}
}
}
}
{
"entity": {
"sample": {
"name": "Test Name"
}
}
}
That is correct, and a fair comment. Yes, it is expected all external prefabs referenced are fully qualified. Would it be better to add a bit more syntax here, such as:I figure the main/only thing that marks the "journal" as internal is the lack of a qualifier in front? Any chance of mixups between internal references and shorthand references to unambiguous prefabs?
"ref" : ["Prefab(core:torch)", "Prefab(torch)", "Local(journal)", "journal"]