Simplified event handling

[GregHib]

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.

[GregHib]

Best options seem to be:

1. Store wildcards and matchers in a trie and use the trie to lookup handlers for every entity on startup, then dispose of the trie keeping a 1:1 map.
2. Group entities by categories for faster lookups (e.g. human, banker, attackable) etc..

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 use on handlers will take up more spaces than individual handlers.

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.

[GregHib]

1. has issues as not all the values are known upfront. E.g. the number of slots for an interface. The number of parameters would have to be cut down significantly
2. only alleviates the problem, it requires adding categories to definitions, which needs doing anyway but should be done first/separately

For now it might be worth focusing instead on untangling the implicit dependencies, removing all the instances of override and generally simplifying Events, might be an idea to separate emit and broadcast to remove that variable from all events.

[GregHib]

Interfaces will be better to use the interface:component format to identify matching against a specific component, or all if no ':' exists.

[GregHib]

With #640 all events should now be untangled.

There is now 2 types of events:

1. Notifications - Broadcast event to multiple subscribers
2. Events - Sent to only one subscriber

And 3 types of handlers:

1. Exact matches
2. Wildcard matches
3. Default matches

Next steps:

1. Use existing trie to make a 1:1 map
2. Improve map by introducing default handlers
3. Store broadcast-able events separately

[GregHib]

Got to watch out for cases where more specific values override less specific but aren't a notification.

E.g.

interfaceOption("Cast", "*_teleport", "*_spellbook")
interfaceOption("Cast", "lumbridge_home_teleport", "modern_spellbook")

[GregHib]

Ran some experiments with #641 using InterfaceOption. Essentially it'd take a good 8-12ms on startup to loop through all of the interface options and add them. And the introduction of a branch in the emit() function adds a bunch of extra overhead. It doesn't seem feasible to have both the flexibility and decent performance across the board.

I had a look at what a explicit list function would look like for interfaces:

fun interfaceOption(option: String, vararg interfaces: String) {
    var previous = ""
    for (component in interfaces) {
        val index = component.indexOf(':')
        if (index == 0) {
            println("PUT ${previous}$component:$option")
        } else if (index != -1) {
            previous = component.take(index)
            println("PUT ${component}:$option")
        } else {
            previous = component
            println("PUT $component::$option")
        }
    }
}

interfaceOption("Use", "my:hands", ":feet") // my:hands, my:feet
interfaceOption("Use", "hands", "feet") // hands, feet
interfaceOption("Use", "my", ":feet") // my, my:feet

It has some niceties of not having to repeat interface names but when you have a long list it doesn't actually look nicer, e.g.

interfaceOption(
    option = "Cast",
    "modern_spellbook:lumbridge_teleport",
    "modern_spellbook:camelot_teleport",
    "modern_spellbook:mobilising_armies_teleport",
    "modern_spellbook:ardougne_teleport",
    "modern_spellbook:falador_teleport",
    "modern_spellbook:varrock_teleport",
    "modern_spellbook:lumbridge_home_teleport",
    "modern_spellbook:watchtower_teleport",
    "modern_spellbook:trollheim_teleport",
    "ancient_spellbook:paddewwa_teleport",
    "ancient_spellbook:lassar_teleport",
    "ancient_spellbook:kharyrll_teleport",
    "ancient_spellbook:annakarl_teleport",
    "ancient_spellbook:senntisten_teleport",
    "ancient_spellbook:dareeyak_teleport",
    "ancient_spellbook:edgeville_home_teleport",
    "ancient_spellbook:carrallanger_teleport",
    "ancient_spellbook:ghorrock_teleport",
    "lunar_spellbook:lunar_home_teleport",
    "lunar_spellbook:ourania_teleport",
    "lunar_spellbook:moonclan_teleport",
    "lunar_spellbook:catherby_teleport",
    "lunar_spellbook:khazard_teleport",
    "lunar_spellbook:barbarian_teleport",
    "lunar_spellbook:waterbirth_teleport",
    "lunar_spellbook:ice_plateau_teleport",
)

interfaceOption(
    option =  "Cast",
    "modern_spellbook:lumbridge_teleport",
    ":camelot_teleport",
    ":mobilising_armies_teleport",
    ":ardougne_teleport",
    ":falador_teleport",
    ":varrock_teleport",
    ":lumbridge_home_teleport",
    ":watchtower_teleport",
    ":filter_teleport",
    ":trollheim_teleport",
    "ancient_spellbook:paddewwa_teleport",
    ":sort_teleport",
    ":lassar_teleport",
    ":kharyrll_teleport",
    ":annakarl_teleport",
    ":senntisten_teleport",
    ":filter_teleport",
    ":dareeyak_teleport",
    ":edgeville_home_teleport",
    ":carrallanger_teleport",
    ":ghorrock_teleport",
    "lunar_spellbook:sort_teleport",
    ":lunar_home_teleport",
    ":ourania_teleport",
    ":moonclan_teleport",
    ":filter_teleport",
    ":catherby_teleport",
    ":khazard_teleport",
    ":barbarian_teleport",
    ":waterbirth_teleport",
    ":ice_plateau_teleport",
)

Some thoughts I've had

• Gradually migrating might be better
• Using explicit names might not be terrible, wildcards are nice but they will be slow regardless
• Filtering using definitions could be a middle ground e.g. interfaceDefinition.components.filter { stringId.startsWith()}
• Storing handlers inside definitions so handler: () -> Unit for individuals, handlers: Array<() -> Unit> for options lists (and use indexes to access) and usedOn: Map<String, () -> Unit> - This only works if handlers are Character, and the handler itself checks and delegates npcs vs players doing the same action, also doesn't work for custom events
• Could limit where wildcards are allowed e.g. in components but not interface ids, or even limit to just * default values for Options
• Removing the single emit() point makes adding debug statements and tracking players events harder

[GregHib]

Need to go through every event to check how best to convert them:

• Q: How best to handle player/npc/character attack npc divides

- Events
NPCOption.kt approach: Map<String, Handler>, operate: Map<String, Handler> String = "npc:option"
    1. Option for all npcs
    2. Option for specific list of wildcarded npcs
    3. Any option for specific list of wildcarded npcs
AiTick.kt // subscriber: Handler
HuntObject.kt
CombatStop.kt
HuntPlayer.kt
TimerStart.kt
CombatStart.kt
RegionRetry.kt
GrantExp.kt
StringEntered.kt
PublicChatMessage.kt
InterfaceOpened.kt
TimerStop.kt
CombatInteraction.kt
ItemOnObject.kt
ObjectOption.kt
ItemOnFloorItem.kt
CancellableEvent.kt
FloorItemOption.kt
ClanChatMessage.kt
InterfaceOption.kt
ReloadRegion.kt
PlayerOption.kt
InterfaceRefreshed.kt
CombatReached.kt
PrivateQuickChatMessage.kt
InterfaceClosed.kt
StartBot.kt
SuspendableEvent.kt
InventoryUpdate.kt
ReloadZone.kt
Interaction.kt
ContinueDialogue.kt
AreaExited.kt
AreaEntered.kt
HuntNPC.kt
TargetInteraction.kt
TimerTick.kt
RegionLoad.kt
CloseInterface.kt
Command.kt
HuntFloorItem.kt
Moved.kt
VariableBitAdded.kt
BlockedExperience.kt
LeaveClanChat.kt
PrivateChatMessage.kt
PublicQuickChatMessage.kt
VariableBitRemoved.kt
InterfaceSwitch.kt
ItemOnNPC.kt
ClanQuickChatMessage.kt
ItemOnItem.kt
ItemOnPlayer.kt
CombatSwing.kt
DropItems.kt
BoughtItem.kt
SoldItem.kt
DoorOpened.kt
SpecialAttack.kt
SpecialAttackDamage.kt
InventoryOption.kt
ItemUsedOnItem.kt
Destroyed.kt
Destructible.kt
Droppable.kt
Dropped.kt
Takeable.kt
Taken.kt
OpenQuestJournal.kt
Consumable.kt
Consume.kt

- Notifications - Easy enough to handle?
VariableSet.kt
SettingsReload.kt
Despawn.kt
Spawn.kt
CurrentLevelChanged.kt
MaxLevelChanged.kt
ItemChanged.kt
CombatPrepare.kt
CombatAttack.kt
CombatDamage.kt
Death.kt
OpenShop.kt
ObjectTeleport.kt
SpecialAttackPrepare.kt
Teleport.kt
PrayerStart.kt
PrayerStop.kt

[GregHib]

CombatInteraction.kt - Set<Handler>
Command.kt - Map<String, Handler>
FloorItemOption.kt - Item groups, option wildcard defaults
InterfaceOption.kt - Interfaces no wildcards, components, options can be wildcarded with defaults
InventoryOption.kt - inventory no wildcards, options wildcard with defaults
ItemOnFloorItem.kt - item groups x2
ItemOnNPC.kt - item groups, npc groups
ItemOnObject.kt - item groups, object groups
ItemOnPlayer.kt - item groups
NPCOption.kt - npc groups, option wildcards
ObjectOption.kt, object groups, option wildcards
OpenQuestJournal.kt - 
PlayerOption.kt - option wildcards

[GregHib]

I've done some more tidying up in #642.

"simplified handlers" are just not better.

The pro's are just this thought of better or purer code, and the O(1) runtime which is only theoretically faster. Because for default cases you have to check the map multiple times it's probably no faster collecting several sets and calling them than doing a single trie search.

And if you wanted to avoid that by making it 1:1 well that only works for events which are 1:1 otherwise complex events like use_on will explode. 60k objects, 20k items, that's 1.2B map entries most of which are junk/default.

But it just doesn't outweigh the cons; It's going to be:

• A lot slower on startup.
• Less user/dev friendly
• More boilerplate code

I tried again converting ItemChanged and it does neaten the handling side of the code

companion object {
        private val additions: MutableMap<String, MutableSet<suspend ItemChanged.(Player) -> Unit>> = Object2ObjectOpenHashMap(128, Hash.VERY_FAST_LOAD_FACTOR)
        private val removals: MutableMap<String, MutableSet<suspend ItemChanged.(Player) -> Unit>> = Object2ObjectOpenHashMap(128, Hash.VERY_FAST_LOAD_FACTOR)

        suspend fun handle(player: Player, change: ItemChanged) {
            handle(player, change, additions, "${change.inventory}:${change.item.id},${change.index}")
            handle(player, change, additions, "${change.inventory}:${change.item.id}")
            handle(player, change, additions, change.inventory)
            handle(player, change, removals, "${change.from}:${change.fromItem.id},${change.fromIndex}")
            handle(player, change, removals, "${change.from}:${change.fromItem.id}")
            handle(player, change, removals, change.from)
        }

        private suspend fun handle(player: Player, change: ItemChanged, map: Map<String, Set<suspend ItemChanged.(Player) -> Unit>>, key: String) {
            val handlers = map[key] ?: return
            for (handler in handlers) {
                handler.invoke(change, player)
            }
        }

        fun trackAdd(key: String, handler: suspend ItemChanged.(Player) -> Unit) {
            additions.getOrPut(key) { ObjectOpenHashSet(12, Hash.VERY_FAST_LOAD_FACTOR) }.add(handler)
        }

        fun trackRemove(key: String, handler: suspend ItemChanged.(Player) -> Unit) {
            removals.getOrPut(key) { ObjectOpenHashSet(12, Hash.VERY_FAST_LOAD_FACTOR) }.add(handler)
        }
    }

But then it will make all of the handlers more convoluted as every item parameter needs a check and lookup for categories/groups

fun itemAdded(item: String = "*", slot: EquipSlot, inventory: String = "*", handler: suspend ItemChanged.(Player) -> Unit) {
    if (item.length > 1 && item[0] == '#') {
        val items = get<ItemDefinitions>().groups[item.removePrefix("#")]
        requireNotNull(items) { "No items found in category '${item.removePrefix("#")}'" }
        for (itm in items) {
            ItemChanged.trackAdd("$inventory:${itm},${slot.index}", handler)
        }
    } else {
        ItemChanged.trackAdd("$inventory:${item},${slot.index}", handler)
    }
}

So it's not even any simpler or neater, if anything it's worse

[GregHib]

What is in place is good enough just need to make sure the events don't get to big or complicated.