[script] [common-items] New helper method paradigm (rough draft, work in progress)

[asechrest]

I've really enjoyed learning the Lich code base. It's an impressive open-source project and for some reason is imminently approachable for someone like me who doesn't code for a living. Maybe it's the Ruby language, or how well-built the code base is. Either way, I'm enjoying it.

This draft is not close to complete and is not meant to be, but I wanted to put something up as a concept for discussion among contributors. I've tossed this around with some other contributors and we at least judged it worthy of discussion. I am soliciting comments and suggestions on this idea.

Background: I love the compartmentalization of the helper methods in the commons. It allows us to centralize the details of game input actions that are extraordinarily common. We centralize the game input and regex recognition, aggregate game output messages, make game actions "safe" so we don't lose items, handle common and benign failures and retry the input, and more.

Current Paradigm: By and large, the current paradigm is that the helper methods take some input, try some action, and return either true or false -- whether the action succeeded or failed. This serves a great purpose - offload the action and the details thereof, and tell us whether we succeeded. Then, the script calling the helper method can take action on a pass or fail basis.

An Example: Let's take an easy example. Currently, common-items has a series of methods that wear an item. It is currently 3 methods, called in a hierarchical pattern, accepting as input the item we want to wear, helping make our input safe by appending "my" to the item, utilizing bput to perform the action, and aggregating all known success and failure patterns for wearing an item. It returns true if we successfully wore the item, and false if we didn't. A calling script can use DRCI.wear_item("boots") and not have to worry about the details of doing so.

The Problem: Not really a problem, but an opportunity. We regularly want to perform a common game action within a script. And the common helper methods are good for this task. But, rather than understanding whether the action simply succeeded or failed, we often want to to understand what the game feedback is and perhaps take specific action within the script calling the helper method. In the example above, the calling script only has knowledge of whether the action succeeded or failed. It has no ability to understand what the game output was, and to take action on certain output. In essence, we have these beautiful centralized helper methods, but they don't help us do what we so often need to do -- understand what the game output was, and handle various different output....differently.

Current Scenario: Where does this leave us? We have a plethora of common-* helper scripts that are great to utilize. But any time we want a calling script to take a common game action and custom handle game output, we're left RE-creating the wheel via a bput, even when that action is something incredibly simple like getting an item. And because we're constantly re-creating the wheel and we have dozens of contributors trying to re-create success and failure messages, we have dozens of places where we have to fix things.

The Idea - 10k Foot View: The idea is to maintain legacy functionality but allow a new paradigm -- create helper methods that still centralize all the details of common game actions but allow a calling script to access the game output, if desired, and take specific action on that output. Instead of simply returning true or false when calling a helper method like DRCI.wear_item, we'll now allow the option to return the full game output line that matched. What does this get us? It still allows us to:

• Centralize the bput and success/failure match strings for common game actions
• Maintain the regular details of very common actions within a single method or set of methods in a Module
• Still allowing flexibility to parse different outputs within a script utilizing said Modules, without needing to recreate the wheel via bput any time we need to take custom action on game output

Concrete Example:

    detailed_output = DRCI.get_item?(item, container, detailed = true)

    case detailed_output.output
    when /already in your inventory/
        echo "Do custom thing in our calling script based on this regex match to game output."
    end

What we're seeing here is that we're utilizing a new paradigm - calling DRCI.get_item with a new arg detailed = true, which exposes the full line of game output that matched the helper method, and allows us to take custom action on it in the calling script.

As a real current example where this would be useful, combat-trainer currently has a match failure in its stow_ammo method when your hands are too full to stow something. The missing match phrase actually exists in the success/failure patterns of DRCI.stow_item, so if we could call that helper method here it would be properly handled. But we can't use the helper method here, because combat-trainer takes specific action on certain feedback to the stow action:

    case bput("stow my #{ammo}", 'You pick up', 'You put your', 'Stow what', 'needs to be tended to be removed')
    when 'You pick up', 'You put your'
      stow_ammo(ammo, quantity - 1)
    when 'Stow what'
      # Bug fix for typo with burgled arrows.
      # Messaging will say "you fire a blunt-tipped arrows" (note the trailing 's')
      # when you actually only shot a singular arrow and the item itself
      # only responds to 'stow arrow' (no 's').
      stow_ammo(ammo.slice(0..-2), quantity) if ammo.end_with?('s')

What if we could utilize DRCI.stow_item, where all our regex patterns are aggregated, while still being able to take action on certain game output in our case/when statement in the calling script? That's where this idea comes in.

In summary: We have these great helper methods that are very useful. Yet our codebase is peppered with bputs that completely overlap with the helper methods because they take some custom action on specific game output and the helper methods don't expose the game output to us.

The idea here is to add a new paradigm -- utilize all the good stuff of the centralized methods, while still allowing custom actions on the resulting game output. As a last note, here is a screenshot of a calling-script utilizing the new paradigm of a helper method, echoing the exposed game output, and taking some custom action on it, while also still functioning as expected using the old paradigm.

[asechrest]

@rpherbig @KatoakDR @MahtraDR @Hiinky @vtcifer @Dartellum

Tagging for comments. Thanks.

[KatoakDR]

Great write-up and suggestion. I'm glad you've enjoyed jumping in to the code base. I believe this approach helps us reduce code duplication among scripts while maintaining the flexibility of reacting to specific game output when desired.

[rpherbig]

cc @rcuhljr

[rpherbig]

One of the big reasons for the helpers is so that script developers can think more about what than how.

What do you think about using symbols (since Ruby doesn't really have an enum construct like, say, C#) instead of game output?

    detailed_output = DRCI.get_item?(item, container, detailed = true)

    case detailed_output.output
    when :already_in_inventory
        echo "Do something specific based on this case"
    end

or

    case DRCI.stow_item(...)
    when :success
      stow_ammo(ammo, quantity - 1)
    when :not_found
      # Bug fix for typo with burgled arrows.
      # Messaging will say "you fire a blunt-tipped arrows" (note the trailing 's')
      # when you actually only shot a singular arrow and the item itself
      # only responds to 'stow arrow' (no 's').
      stow_ammo(ammo.slice(0..-2), quantity) if ammo.end_with?('s')

[KatoakDR]

One of the big reasons for the helpers is so that script developers can think more about what than how.

What do you think about using symbols (since Ruby doesn't really have an enum construct like, say, C#) instead of game output?

That's a cool idea too! This further removes the scripts having variations of the specific game text to maintain and instead reference constants that mean each specific scenario a match string(a) is for.

[KatoakDR]

After some thought, I like having both options that @rpherbig and @asechrest propose: the error category and the low-level game message, with the priority for devs to use the util methods the following ways:

1. DRCI.get_item?(name) -- true/false (preferred)
2. DRCI.get_item?(name, true)[:hands_full] -- inspect the reason/category in a consistent way, specific details still abstracted; multiple match strings may be this category
3. DRCI.get_item?(name, true)[:output] -- the actual matched game message, only use if absolutely necessary; PRs would recommend folks to use style 1 or 2 unless 3 is only way to do it

[asechrest]

I like those ideas, too.

I think categorizing the failures allows some granularity to the handling in the calling scripts, and I think allowing access, if absolutely necessary, to the actual game output line that matched, provides an ability to customize the handling without fully re-recreating 99% of the helper method via bput.

[rpherbig]

@KatoakDR one thing, I don't think the versions that return a struct should have ? in their name (since they're no longer really predicate functions). It also allows us to make a ? version for most uses and a non-? version only when needed.

In terms of the how we go about changing this, I would suggest doing it one function at a time. We don't need to overhaul the entire codebase if versions 2 and 3 of a function aren't needed. But having the pattern in hand for when we do is great!