Plugin API ergonomics about apollo HOT 8 OPEN

What about reducing the amount of chaining necessary? We might be offering too much flexibility.

For example, this is a collection of examples on how buttons are handled at the moment:

on { ButtonMessage::class }
    .where { widgetId in Emote.MAP }
    .then { player ->
        player.playAnimation(Emote.fromButton(widgetId)!!.animation)
    }

on_button(LOGOUT_BUTTON_ID)
    .where { widgetId == LOGOUT_BUTTON_ID }
    .then { it.logout() }

on { ButtonMessage::class }
    .where { widgetId == WALK_BUTTON_ID || widgetId == RUN_BUTTON_ID }
    .then {
        it.toggleRunning()
    }

These are the most common patterns. Maps of widget IDs to other types, or simple comparisons against a few widget IDs.

Here's some shorter examples:

on_button(Emote.MAP) {
    player.playAnimation(Emote.fromButton(widgetId)!!.animation)
}

on_button(LOGOUT_BUTTON_ID) {
    it.logout()
}

on_button(WALK_BUTTON_ID, RUN_BUTTON_ID) {
    it.toggleRunning()
}

This is quite a bit cleaner for all those cases. By providing a few new simple overloads we can get rid of a bunch of boilerplate and reduce the level of indentation by 1.
We can take this further for other message types and interactions.

Object actions. This might even be cleaner if we wired up creation of the Action to the method invocation somehow, same with itens.

old

on { ObjectActionMessage::class }
    .where { option == 2 && id in DUMMY_IDS }
    .then { DummyAction.start(this, it, position) }

new

on_object_action(DUMMY_IDS, option = 2) {
    DummyAction.start(this, it, position)
}

Player events.

old

on_event { MobPositionUpdateEvent::class }
    .where { mob is Player }
    .then {
        for ((area, action) in actions) {
            if (mob.position in area) {
                if (next in area) {
                    action.inside(mob as Player, next)
                } else {
                    action.exit(mob as Player, next)
                }
            } else if (next in area) {
                action.entrance(mob as Player, next)
            }
        }
    }

new

on_player_event<MobPositionUpdateEvent> {
    for ((area, action) in actions) {
        if (mob.position in area) {
            if (next in area) {
                action.inside(mob as Player, next)
            } else {
                action.exit(mob as Player, next)
            }
        } else if (next in area) {
            action.entrance(mob as Player, next)
        }
    }
}

from apollo.

MessageHandlers have a lot of room for improvement, especially ones like ItemOnItem:

on { ItemOnItemMessage::class }
    .where { usedId == SOME_ID && targetId == SOME_OTHER_ID || 
        usedId == SOME_OTHER_ID && targetId == SOME_ID
     }
    .then { player -> ... }

(Almost?) every ItemOnItem handler is bidirectional, so ergonomic improvements here would be welcome.

Other examples would include looking up players/objects/npcs when handling frames that reference their index (like NpcOptionMessage or ItemOnObjectMessage).

from apollo.

Utility methods to deduce the indefinite article (a/an) for a string (this already exists in core in LanguageUtil but we probably want to expose this to plugins some other way).

from apollo.

On getting rid of the concept of "messages" from plugins, these are the current calls to on:

Function
    on(() -> KClass<T>)
Found usages  (32 usages found)
    bank.plugin.kts  (2 usages found)
        15 on { ObjectActionMessage::class }
        22 on { NpcActionMessage::class }
    consumables.plugin.kts  (1 usage found)
        9 on { ItemOptionMessage::class }
    door.plugin.kts  (1 usage found)
        9 on { ObjectActionMessage::class }
    dummy.plugin.kts  (1 usage found)
        15 on { ObjectActionMessage::class }
    EmoteTab.plugin.kts  (1 usage found)
        3 on { ButtonMessage::class }
    Fishing.plugin.kts  (1 usage found)
        10 on { NpcActionMessage::class }
    friends.plugin.kts  (1 usage found)
        5 on { AddFriendMessage::class }
    Herblore.plugin.kts  (4 usages found)
        11 on { ItemOptionMessage::class }
        20 on { ItemOnItemMessage::class }
        30 on { ItemOnItemMessage::class }
        40 on { ItemOnItemMessage::class }
    ignores.plugin.kts  (2 usages found)
        4 on { AddIgnoreMessage::class }
        7 on { RemoveIgnoreMessage::class }
    KotlinPluginScript.kt  (1 usage found)
        49 fun on_button(id: Int) = on { ButtonMessage::class }.where { widgetId == id }
    messaging.plugin.kts  (1 usage found)
        7 on { PrivateChatMessage::class }
    Mining.plugin.kts  (2 usages found)
        4 on { ObjectActionMessage::class }
        12 on { ObjectActionMessage::class }
    PlayerActions.plugin.kts  (1 usage found)
        6 on { PlayerActionMessage::class }
    Prayer.plugin.kts  (2 usages found)
        14 on { ButtonMessage::class }
        30 on { ItemOptionMessage::class }
    run.plugin.kts  (1 usage found)
        6 on { ButtonMessage::class }
    Runecrafting.plugin.kts  (7 usages found)
        28 on { ObjectActionMessage::class }
        40 on { ItemActionMessage::class }
        49 on { ItemOnObjectMessage::class }
        58 on { ItemOptionMessage::class }
        67 on { ItemOnObjectMessage::class }
        76 on { ObjectActionMessage::class }
        85 on { ObjectActionMessage::class }
    shop.plugin.kts  (2 usages found)
        23 on { NpcActionMessage::class }
        35 on { ItemActionMessage::class }
    Woodcutting.plugin.kts  (1 usage found)
        17 on { ObjectActionMessage::class }

from apollo.

Using inline reified functions in the on function would remove the explicit need to call ::class from the messages. I find this a welcome addition. I'm not sure of another way to do this nicely without creating an additional function per Message

New syntax would be something along the lines of this:

on<NpcActionMessage>().where { predicate }.then { consumer }

Thoughts?

from apollo.

On second thought, maybe we could get rid of the predefined predicate to clean up plugin code even more, I believe we discussed this change before and came to the consensus that the current predicate only existed for event termination, which I believe we also determined to be potentially problematic.

If we went this route, we could supply the predicate inside the consumed event itself, proposed syntax would be something along the lines of:

on<NpcActionMessage> {
    return@on if (predicate)
    // .. consume message ..
}

from apollo.

Using inline reified functions in the on function would remove the explicit need to call ::class from the messages.

This was considered initially, but I went with the current form because of the consistency with the lambdas.

If we went this route, we could supply the predicate inside the consumed event itself

I think the current return@blah stuff is ugly enough without this. Perhaps if we could get rid of the @...

from apollo.

I think we can get rid of the return@label by using an anonymous function (ugly) or by marking the consumer as inline. I was mistaken, inlining is the problem here.. Hmm.

from apollo.