Skip to content

Conversation

@viciscat
Copy link
Collaborator

It's not a new feature D:

A class that allows for parsing of the abilities of an item! wowi!
Might be unnecessary but has masks to only parse the needed things and save on resources.

@LifeIsAParadox LifeIsAParadox added the reviews needed This PR needs reviews label Dec 14, 2025
@Alex33856
Copy link
Contributor

It's a new feature :D

adding new feature label

@Alex33856 Alex33856 added the new feature This issue or PR is a new feature label Dec 14, 2025
@viciscat
Copy link
Collaborator Author

you scaly wag

@LifeIsAParadox LifeIsAParadox added merge conflicts This PR has merge conflicts that need solving. and removed reviews needed This PR needs reviews labels Dec 20, 2025
@LifeIsAParadox LifeIsAParadox added reviews needed This PR needs reviews and removed merge conflicts This PR has merge conflicts that need solving. labels Dec 20, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces an ItemAbility API for parsing and caching item abilities from Skyblock item lore, refactoring existing code to use this centralized approach instead of scattered regex-based parsing.

Key Changes:

  • Adds new ItemAbility record class with ability name, activation type, and optional costs/cooldown
  • Caches parsed abilities in ItemStack via mixin for performance
  • Refactors four existing features to use the new API instead of custom regex patterns

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
src/main/java/de/hysky/skyblocker/utils/ItemAbility.java New API class with regex patterns for parsing ability name, activation type, mana/soulflow costs, and cooldown from item lore
src/main/java/de/hysky/skyblocker/skyblock/teleport/PredictiveSmoothAOTE.java Refactored to use ItemAbility API for mana cost checking instead of direct lore regex matching
src/main/java/de/hysky/skyblocker/skyblock/dwarven/PickobulusHelper.java Simplified ability checking to use ItemAbility.hasAbility() method
src/main/java/de/hysky/skyblocker/skyblock/SwingAnimation.java Refactored to check parsed abilities for right-click activation instead of regex matching
src/main/java/de/hysky/skyblocker/skyblock/StatusBarTracker.java Updated mana tracking to iterate through parsed abilities instead of searching lore text
src/main/java/de/hysky/skyblocker/mixins/ItemStackMixin.java Added caching field and getter method for abilities in ItemStack mixin
src/main/java/de/hysky/skyblocker/injected/SkyblockerStack.java Added skyblocker$getAbilities() interface method with empty list default

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

return true;
List<ItemAbility> abilities = stack.skyblocker$getAbilities();
for (ItemAbility ability : abilities) {
if (ability.activation() == ItemAbility.Activation.RIGHT_CLICK) return true;
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

The refactored logic changes behavior from the original implementation. The old regex pattern matched any ability ending with "right click", which would include both RIGHT_CLICK and SNEAK_RIGHT_CLICK. The new code only checks for exact RIGHT_CLICK activation, missing SNEAK_RIGHT_CLICK abilities. This could cause swing animations to not display for items with sneak right-click abilities.

Suggested change
if (ability.activation() == ItemAbility.Activation.RIGHT_CLICK) return true;
if (ability.activation().name().endsWith("RIGHT_CLICK")) return true;

Copilot uses AI. Check for mistakes.
if (foundRightClick && (matcher = MANA_LORE.matcher(text)).matches()) {
manaCost = RegexUtils.parseIntFromMatcher(matcher, 1);
for (ItemAbility ability : handStack.skyblocker$getAbilities()) {
if (ability.activation() == ItemAbility.Activation.RIGHT_CLICK) {
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

The refactored logic changes behavior from the original implementation. The old code matched any ability text ending with "right click" (case-insensitive), which would include both "RIGHT CLICK" and "SNEAK RIGHT CLICK". The new code only checks for exact RIGHT_CLICK activation, potentially missing mana costs for SNEAK_RIGHT_CLICK abilities. This could cause incorrect mana tracking for items with sneak right-click abilities.

Suggested change
if (ability.activation() == ItemAbility.Activation.RIGHT_CLICK) {
if (ability.activation() == ItemAbility.Activation.RIGHT_CLICK
|| ability.activation() == ItemAbility.Activation.SNEAK_RIGHT_CLICK) {

Copilot uses AI. Check for mistakes.
@kevinthegreat1
Copy link
Collaborator

@viciscat I remember you responded somewhere but can't find it. Those concerns aren't an issue right?

@viciscat
Copy link
Collaborator Author

I don't remember responding. One of us is crazy. It could be ignored, the amount of shift right click abilities seems really small. But it can be fixed by doing an || with SHIFT_RIGHT_CLICK or adding a small method to the enum to make it a little cleaner

@Alex33856 Alex33856 added this to the 6.0.0 milestone Dec 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new feature This issue or PR is a new feature reviews needed This PR needs reviews

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants