Pushes, pulls and charges, which nothing read - #2309
Open
denislauri1999 wants to merge 1 commit into
Open
Conversation
BCard type 40. Three of its subtypes are used by the game's skills, 134
declarations between them, and all three move somebody:
11: Push your opponent back %s fields. 49
21: Draws enemies to %s fields away from you. 64
31: Charge at enemies within %s fields. 21
SUBTYPE 21 IS A PULL, NOT A TAUNT, and the enum said otherwise: it was called
FocusEnemies. The file says "Draws enemies to %s fields away from you", the
value is a distance of 0 to 4 across the whole file - 25 of the 64 say zero,
which is "up against you" - and the skills carrying it are called Drawing Shot,
Rotating Hammer and Spider King's Draw. A taunt has nothing to do with a number
of fields. Renamed to DrawEnemies.
The rule is the same for all three: walk cell by cell and stop in front of the
first obstacle. Jumping to the destination would put somebody inside a wall or
past the edge of the map, and from there they cannot get out - client and server
would disagree about where they are.
The geometry is its own class rather than a private method, because it is the
part that can be wrong quietly, and because the part around it can only move a
real ECS bundle: the position lives on a component and the interface exposes it
read-only, so a test double would never budge and would prove nothing.
Subtypes 41 "Run Away!" and 51 "Hide" are declared by no skill in the file. They
are not here: there would be nothing to check them against.
Ignoring the wall fails three tests, stopping one cell short fails three.
|
Warning Review limit reachedNext included review available in 16 minutes. View limit detailsLimit details: You’ve used all 2 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
BCard type 40. Three of its subtypes are used by the game's skills — 134 declarations between them — and all three move somebody:
Subtype 21 is a pull, not a taunt
The enum called it
FocusEnemies. The file says "Draws enemies to %s fields away from you", the value is a distance — 0 to 4 across the whole file, and 25 of the 64 say zero, which is "up against you" — and the skills carrying it are named Drawing Shot, Rotating Hammer and Spider King's Draw.A taunt has nothing to do with a number of fields. Renamed to
DrawEnemies.The rule is the same for all three
Walk cell by cell and stop in front of the first obstacle. Jumping to the destination would put somebody inside a wall or past the edge of the map, and from there they cannot get out — the client and the server would stop agreeing about where they are.
The geometry is its own class rather than a private method, for two reasons: it is the part that can be wrong quietly (a step too many puts you in a wall, a step too few makes a charge stop short), and the part around it can only move a real ECS bundle — the position lives on a component and the interface exposes it read-only, so a test double would never budge and would prove nothing.
Left out, with the reason
Subtypes 41 "Run Away!" and 51 "Hide" are declared by no skill in the file. There would be nothing to check an implementation against.
Checked by breaking it
Ignoring the wall fails three tests; stopping one cell short of the named distance fails three.
TryApplyHitbecomesasync, the same change #2304 makes for the same reason — I gave both the identical comment so the two merge cleanly rather than colliding on a difference that means nothing.