-
Notifications
You must be signed in to change notification settings - Fork 24
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
Simplified event handling #582
Comments
Best options seem to be:
That should minimise the impact on startup, assuming it's negligible the deciding question would be how much extra memory would the 1:1 map take-up. This will be hard to estimate as It would also depend on whether there's a default handler. For example "Attack" is handled by a single entry, instead of adding it to all 30k npcs. |
For now it might be worth focusing instead on untangling the implicit dependencies, removing all the instances of |
Interfaces will be better to use the |
* Extract light source for reuse * Add help command * Extract SpellRunes from Spell.kt * Add commands to find * Fix coal bag * Move agility test package * Add crafting jewellery tests * Crafting tweaks * Add smelting tests #583 * Add tab helper method * Add easier sendQuestComplete method * Rename questComplete to questCompleted * Rename quest scroll interface methods * Rename CombatHit to CombatDamage * Rename weapon animations block to defend * Rename npc _hit animations to _defend * Rename weapon animations from _hint to _defend * Rename sounds from _hit to _defend * Rename remaining spell _hit's to _impact * Make Item amount default to 1 * Rename SpecialAttackHit to Damage * Rename Pickup to Take, and add Taken events to remove overrides from FloorItemOption #582 * Fix suspendable target delay and dialogues * Fix blank statements * Fix varrock manhole ladder * Fix teleport by some names * Fix account storage test
Events are currently stored in a Trie which is a middle ground between efficiency, complexity and flexibility. See #324
Having a 1:1 relationship between possible actions and handlers should be the simplest solution and give the best possible performance, flexibility doesn't matter as much as this is how runescript already handles it.
Being overly flexible in the past has allowed complexity in behaviour where one piece of content indirectly overrides another. This is great for modularity but adds implicit dependencies between things, combat being the main example.
Ideally the event system should not be dependent on overriding or order.
Untying
Attempts were made in #324 to reduce the amount of interdependency but still some content remains that requires implicit order of execution dependencies on others. This needs removing before moving to a different system.
Matching
The trade off to be made is how and when to match a handler with an actor. In a pure 1:1 world, the developer would have to manually write a setter for every single entity. With entity counts being approx 200k this is impractical especially as handlers are shared and reused often e.g. nearly all bankers have the same dialogue.
So there must be a way to match multiple entities, the current system uses id Wildcards. This is fine except storing all of the wildcards and checking them against an entity at runtime isn't as efficient as it could be, although a trie is implemented to be faster than looping every wildcard it has added complexity to every event.
The obvious alternative is to filter the list of entities using the wildcard on startup, e.g. to fill in a Map<Entity, Handler>.
The issue with this is performance as a naive approach of checking every entity against every wildcard will be slow. E.g. 100 wildcards tested against 50k entities is 5m iterations.
The current handler count is at just under 2k, and will grow substantially as more content is added.
The text was updated successfully, but these errors were encountered: