Skip to content
This repository has been archived by the owner on Jul 2, 2019. It is now read-only.

Impose order on invoke() selection #104

Open
scionaltera opened this issue Aug 14, 2018 · 0 comments
Open

Impose order on invoke() selection #104

scionaltera opened this issue Aug 14, 2018 · 0 comments
Labels
bug Something isn't working tech debt Code improvements and refactors

Comments

@scionaltera
Copy link
Owner

You can see this happen with PURGE: there are two implementations of invoke() and it's supposed to try binding your arguments to each implementation, find the best match and run that.

What actually happens is it tries to run both and displays both an error message and the successful output from the command. Less than ideal.

Another more subtle issue here is that there's no way to tell PURGE that (for example) you'd prefer it try to bind to items in your inventory before items on the floor. What if there were items that match your argument in both places? Which one is it going to choose first? The answer right now is that it's completely arbitrary and up to Java reflection to decide!

So there are two acceptance criteria for this issue:

  • Invoking a command should never produce two distinct sets of output (e.g. an error message and a success message glued together). Only show an error message to the user if none of the invoke() implementations could successfully bind.
  • There needs to be a way for the programmer to specify which order the implementations are tried. Some commands will have variations based on whether a target is worn, in your inventory, on the ground, or possibly somewhere else in the game. You will almost always want the command to do the smallest-scoped search first and progress to the largest scope last - both for efficiency and because that's what would generally make the most sense to the player.
@scionaltera scionaltera added bug Something isn't working tech debt Code improvements and refactors labels Aug 14, 2018
@scionaltera scionaltera changed the title Impose order on invoke() implementations Impose order on invoke() selection Aug 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working tech debt Code improvements and refactors
Projects
None yet
Development

No branches or pull requests

1 participant