Skip to content
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

Create SpellRequirement for easier spell manipulation. #187

Open
wants to merge 66 commits into
base: master
Choose a base branch
from

Conversation

Senmori
Copy link
Contributor

@Senmori Senmori commented Feb 5, 2021

This adds a SpellRequirement that makes it possible to dictate which spell(s) a player should cast (for whatever reason).

Spell Requirement
SpellRequirement is created via the MagicSpell interface. This interface has all the required information about the spell so the requirement can display everything and highlight what needs to be highlighted.

Because MagicSpell is just an interface it is implemented (for now) by StandardSpell. These are the normal spells a player has.
In the future, if there is a need, Ancient, Lunar, and Arceuus spells can be easily added by implementing the MagicSpell interface.

StandardSpell.FIRE_SURGE.getSpellRequirement() creates a requirement that checks for the required items needed for a single cast of this spell.
StandardSpell.FIRE_SURGE.getSpellRequirement(int numberOfCasts) creates a requirement that checks for the required items needed for X number of casts. Where X is the supplied numberOfCasts.

Spell Icons on Quest Steps
Additionally, Queststep now has a addSpell method that takes a MagicSpell parameter.
This let's any step dictate when a spell should be used and who/what it should be used on.
(i.e. Use Telekinetic Grab on a ground item)

Bank Filters
There is now an additional configuration option to select if you want to prefer runes or staves to be shown if there is a spell requirement. Right now it is hidden, but it defaults to RUNES.

API:
BankItemHolder: This is an interface that lets anything (just SpellRequirement for now) define which ItemRequirement should be shown in the quest bank tab.
I don't see many reasons this should be used over the existing alternate id method we use now, however SpellRequirement was a niche scenario but I wanted to be able to easily add this functionality into other requirements if they needed it.
Although I doubt there would be a good reason to prefer BankItemHolder over just having alternate item ids.

Update 10 FEB 2021:
Originally, BankItemHolder was called with the Client, a boolean whether to consider the player's inventory and a list of Items.
This has been replaced with Client, QuestHelperConfig. This is because of the helper class that has been created to handle item searching on players.

ItemSearch
This utility class handles the checking if a player has an item anywhere we can provide an InventoryID. This allows us to not have to rely upon BankItem being null, we just have to determine if it's appropriate to check for an item in a specified location.

There are methods for every type of inventory QuestHelper uses (inventory, equipment, and bank). As well as seeing if an item is simply in those containers or if there is a specified amount of items in those container.

Additionally, the count methods (checkItem, getItemAmountExact) both use longs as their return type. This is because, at least theoretically, you could ask how many coins a player has in their inventory and bank and get a number larger than an integer.
I doubt we'll run into this issue with longs. If so then we deserve it for underestimating players.

The convenience method to see if a player has an item anywhere (inventory, equipment, bank) is hasItemAnywhere. And to check if they have any item with a specified amount is hasItemAmountAnywhere.

findFirstItem will return the first item id out of a given collection of ids. It will return -1 if it does find any of those ids.
getItemAmountExact will return the number of items in a given container that match the given item id.
If the given item container is null it will return 0.

There are also convenience methods for using ItemRequirement to see where a player has item(s).
It check if it's on the player or if it's in the player's bank.
hasItems... are the methods for those.

You can also check to see how many of an item a player has in a given container if you need to perform a check that way.
getItemCount, getItemCountOnPlayer, and getItemCountInBank all get the quantity of an item that matches the given item id. This returns a long as well.

Most of these methods are built on top of checkItem(Client, InventorySlots, int) and getItemAmountExact(Client, InventoryID, int). If any of the above methods are not sufficient these should.

RuneRequirement: This is a helper requirement that holds the rune, how many runes per cast, and the total number of rune(s) needed for X number of casts.
At first, this was an ItemRequirements, however after some testing it very tedious to try and separate the runes and staves from each other while trying to maintain a defined priority (runes over staves at the moment).
Runes are prioritized over staves because not every rune has a staff equivalent.

ItemRequirement#getUpdatedTooltip(Client): This is now available on all ItemRequirements. This should allow us to create better tooltips without having to resort to some weird behavior.
If the returned string is null or empty, the tooltip button (INFO_ICON) is hidden automatically.
The tooltips do support simple html tags. They art wrapped in <html><body style='...'> tags so you do not need to specify those.
As well, all \n characters are replaced with <br> tags so it makes organizing text a bit easier.

StandardSpellBuilder: This should probably be changed to just SpellBuilder in the future because it can build anything that implements MagicSpell. However, there is only standard spells for now.
This is a builder class that is used for chaining calls to determine what runes, items, quests, or other requirements a spell might have in order to be cast.
For example, ARDOUGNE_TELEPORT can only be cast when PLAGUE_CITY is completed. This restriction even applies to teleport tablets.

Rune
This is an enumeration of all Runes in the game.
It contains the rune's name, the runes it represents (i.e. combination runes can represent more than just themselves) as well as the staves that can supply infinite sources of that rune.

Staff
This was originally a list in Rune. However due to it being very tedious to check if a staff can supply a source of runes it was made into it's own enum.
These also contain a list of staves that can supply that type of runes, as well as the runes they can act as a source of.

Fixed MUD rune not actually using MUD_RUNE

Signed-off-by: Senmori <[email protected]>
Have to implement requirement checking, building overlay text, and highlighting widget icons, npcs, objects, etc.

Signed-off-by: Senmori <[email protected]>
… an empty name for a requirement we might display.

Signed-off-by: Senmori <[email protected]>
� Conflicts:
�	src/main/java/com/questhelper/ItemCollections.java
�	src/main/java/com/questhelper/requirements/ItemRequirement.java
Add Spellbook requirement

Signed-off-by: Senmori <[email protected]>
Add Spellbook requirement

Signed-off-by: Senmori <[email protected]>
Also the coloring is wrong.

Signed-off-by: Senmori <[email protected]>
…orrect colors in the overlay and side panel.

Signed-off-by: Senmori <[email protected]>
Use FontMetrics to better place text.

Signed-off-by: Senmori <[email protected]>
…d require the player to use runes/staves.

Signed-off-by: Senmori <[email protected]>
Add live updating of requirements.
SpellRequirement will display the requirements that the user has met as strikethrough text.

Signed-off-by: Senmori <[email protected]>
Signed-off-by: Senmori <[email protected]>
� Conflicts:
�	src/main/java/com/questhelper/ItemCollections.java
Remove debug messages and switch back to using the config to find our bank filter preferences.
� Conflicts:
�	src/main/java/com/questhelper/QuestHelperConfig.java
�	src/main/java/com/questhelper/steps/NpcStep.java
Users can add spell icons to a QuestStep via QuestStep#addSpell.
Add staff builder method to specify explicitly the staff required in order to cast this spell.
Also added a skill method to indicate a non-magic skill required.
…an item id.

Set the name of all blank item requirements.
This will default to the first item id found in the requirement if the player has none of the items listed.
Copy link
Contributor Author

@Senmori Senmori left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove forwarding method calls. Users can use addSpell on QuestStep.

� Conflicts:
�	src/main/java/com/questhelper/requirements/item/ItemRequirement.java
SpellRequirements now color correctly both on the side panel and the overlay panel.
SpellRequirement overlay text is colored separately to give the best indication of what is missing from a spell requirement.
Added an update requirement panel call when opening banks to ensure we update the requirements accordingly.
Added methods to exclusively get the colors for overlays so we don't have to share logic between side panels and overlays.
Also removed the staff requirement from item overlay colors since it has it's own check.
Overlay and side panel will now color correctly.
They both use the same method for determining the color that should be displayed.
Rune and Staff both use suppliers for their references to the other enum (Rune/Staff respectively) so we don't have class-loading issues.
Moved QuestRequirementPanel to use StringUtils instead of a simple null and blank check.
Add a tablet check for overlay panel text.
Removed TriFunction and moved SpellComponentPreference to overriding the getPreference method in the actual enum constant.
Added more convenience methods in ItemSearch to account for use using BankItems almost exclusively.
� Conflicts:
�	src/main/java/com/questhelper/QuestHelperConfig.java
@Zoinkwiz
Copy link
Owner

Just did some testing again, it seems there's a few issues with recommended item not working correctly, not detecting if you have the needed items or not, and a few other things. The process I went through was:

I had a step which requires Wind Blast (1 death 3 air runes).

  • At first it didn't recommend in the bank any air items, just the death rune.
  • I took out some death runes and it updated to recommend an air staff, and adds it to the overlay item reqs
  • I take out some air runes, but it doesn't register as me having the items needed for wind blast
  • If I take out the air staff and drops it, it now still say I need an air staff, but the bank items is also showing the runes. Unfortunately taking out as many death and air runes at this stage still results in the 'Wind Blast' text being red

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants