project-slippi / slippi-js

Parse slp files and compute stats
GNU Lesser General Public License v3.0
147 stars 78 forks source link

Fix Kirby's Neutral B creating additional conversions (closes #85) #90

Closed CMarah closed 2 years ago

CMarah commented 2 years ago

Fixes #85

In stats/common.ts, COMMAND_GRAB_RANGE3 corresponds to action_ids which are not general action states. Those IDs are used for character-specific states, one of which corresponds to Kirby's neutral B. This means that whenever Kirby hits neutral B, a new conversion is created, as the code flagged Kirby as in a command-grabbed state. This conversion appeared as Kirby taking 0% damage.

This bug probably extends to multiple other characters and situations.

This info is based on the list of action_ids found in: https://github.com/project-slippi/slippi-wiki/blob/master/SPEC.md

CMarah commented 2 years ago

In depth explanation:

Melee uses numbers called state ids to internally represent the state of each player. Something very important is that there is NO ambiguity for each individual character / state id pair. Given a state id and a character, there is only one possible in-game state they can be at. What IS possible is for two different characters to have the same state id but be in different states, as Melee reuses some states for different characters.

State ids corresponding to general states, common to every character, are in the range 0 - 340 (0x154). These are states like Wait, Fall, Turn and other more particular ones like ItemScopeRapidEmpty or ShieldBreakStandU. ANY character can be in these states. If a player has state id in this range, you can uniquely determine their actual state, independently of their character. In particular, consider the states

  // Command Grabs
  BARREL_WAIT = 0x125,
  COMMAND_GRAB_RANGE1_START = 0x10a,
  COMMAND_GRAB_RANGE1_END = 0x130,

  COMMAND_GRAB_RANGE2_START = 0x147,
  COMMAND_GRAB_RANGE2_END = 0x152,

If the player has state id among these values, it means they are being command grabbed, and slippi-js correctly creates a conversion to reflect that.

Beyond ID nº 340, Melee starts reusing IDs. From ID 341 onwards there are still general states, but they are NOT reachable in-game. State 341, for example, is labeled Wait1, but you can never be in this state. At this ID range we start ONLY having character-specific states. For example, ID 341 for Fox corresponds to the Blaster Ground Startup state, while ID 341 for Marth corresponds to the Shield Breaker Ground Start Charge state.

The bug fixed by this PR is that slippi-js considers the states in the range 375-382 (range3) as general states, in particular they are considered command-grab states. As an example, relevant to the issue: general state 377, ThrownKoopaEndB, is not reachable in-game. But 377 is a character-specific state ID, corresponding to, among others, Kirby's Swallow Air Capture Wait state. This means that whenever Kirby hits its Neutral B while in the air, it reaches state id 377 at some point. Slippi-js detects this, and mistakenly thinks Kirby is being command grabbed, as 377 is inside range3. This creates a conversion when it shouldn't. The exact same happens with DK. When DK uses his uncharged punch while in the air, it reaches state id 377: Giant Punch Air Early Punch. Once again, Slippi-js would flag this as him being command-grabbed and would wrongly add a conversion.