Fix issues with Auto Shadow Spell #3292
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
the code was not taking into account cases where the player simply didn't had a skill in reproduce or clone slot.
Due to the skill id being stored in val1, the default logic for SCs is that lower val1 values doesn't replace higher ones, so recasting Auto Shadow Spell to use another skill which has a smaller skill_id was failing, while it should be allowed.
guilherme-gm
force-pushed
the
fix-shadow-spell
branch
from
fe0fc82
to
e5eb90c
Compare
4144
requested changes
Apr 28, 2024
…g error reporting Sometimes you want to check whether the skill exists, but its non-existance is not an error for Herc
older clients allows you to click "ok" even when you have not selected a skill for Auto Shadow Spell. In those cases, the client sends a random SkillID which Hercules reports as an internal error (unexpected skill). This is outside our control, but they generate noise and confusion. Checking if the received skill_id is valid before going forward allows us to only let valid skills get into the actuall skill code.
it should always be 1 (meaning auto shadow spell) and not the number of skills in the list
guilherme-gm
force-pushed
the
fix-shadow-spell
branch
from
e5eb90c
to
15710bc
Compare
MishimaHaruna
approved these changes
Apr 30, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request Prelude
Changes Proposed
Fixes a few issues with Auto Shadow Spell.
When casting Auto Shadow Spell, we now properly check if there are skills copied with Intimidade/Reproduce before trying to get the skill_id
This prevents assertion failures due to the skill id being "0" (no skill)
In current code, this would still work, but cause a log about bad skill handling
Fixed a check that prevented you from switching an active shadow spell to another when their Skill ID was smaller (due to sc val1)
Fix assertion error when clicking "cance" or clicking "ok" in Auto Spell list without selecting a skill
As far as I could analyze, the client is not cleaning up the field that stores the current selected skill, so when you click "ok" it just sends a garbage id that may (or may not) be a valid skill. We now validate before running the skill code.
For cancel, it sends id "0", which is also fixed now since 0 is not a valid skill id.
As a bonus, I've converted both packets related to that flow to use structs.
Issues addressed: Closes #3286