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

Abbreviate long input commands #16

Open
wants to merge 48 commits into
base: main
Choose a base branch
from

Conversation

jmarkman
Copy link

End users are used to and appreciate reading abbreviations of stances, i.e., "stand(ing) medium punch" or "stand mp" becomes "st. mp". This pull request refactors the code used for renaming moves based on the user's app display settings, which includes additional code for creating the specified abbreviations.

It also includes some minor refactoring in regards to what's passed to the frame data formatting function (passing the state object for textual display settings instead of passing each property of the state object), code convention, and some variable renaming.

Fixes #9

jmarkman added 11 commits March 9, 2021 20:05
- replace if-else chain with switch statement
- wrap lodash._mapKeys in function
- add empty function as placeholder for shorthand renaming
- apply vs code "format document"
- add "shorthand" option to naming types in settings
d4rk and i discussed the actual implementation of the
updated function into the app and his vision was to have
an extra entirely separate setting for longform and
shorthand notation, so i've adjusted the function a bit
to prepare for him adding a redux store for that setting
and some various other changes to support it
- helpCreateFrameDataJSON now takes in the display state as a parameter
- clarification: renamed stateToSet -> vTriggerStateToSet
- removed select option I added to 'Move Name Type'
dropdown in settings, was using for testing and I swept it
up into the commit
- uncommented d4rk's notation widget for shorthand/longform naming
- renameData func now accepts the display state as an arg,
updated interior of function to leverage it
- updated function docstring for renameData to reflect changes
- 'change displayState' to 'dataDisplayState'
Copy link
Owner

@D4RKONION D4RKONION left a comment

Choose a reason for hiding this comment

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

Great work on this! Your code looks a lot better than mine and I defo should add that @params thing to more of my work haha.

Just a few things that need addressing before we can merge. Please make sure that

Official move names can be shorthanded
Common names can be shorthanded
Inputs can not be shorthanded

src/js/utils/index.ts Outdated Show resolved Hide resolved
src/js/utils/index.ts Show resolved Hide resolved
src/js/utils/index.ts Outdated Show resolved Hide resolved
src/js/utils/index.ts Outdated Show resolved Hide resolved
- ensured that input name types won't be converted to shorthand
- remove ternary operator in return statement in helpCreateFrameDataJSON
@D4RKONION
Copy link
Owner

D4RKONION commented Mar 17, 2021

We have another problem I'm afraid!

Official
-Full Word✅
-shorthand ✅

Common
-Full Word ✅
-shorthand ❌ [Only normals are getting changed, nothing else is being renamed to common names (command normals, sepcials, supers]

Inputs
-Full Word✅
-shorthand✅

- fix bug where common inputs with shorthand were
leaving specials "untranslated" to common input
- refactor formatMoveName to work with moveName
property instead of numCmd property due to consistency
changes in the way numeric inputs were represented
@D4RKONION
Copy link
Owner

So close I can taste it! Just need to account for normals with extra words in them like (Hold) and (Release), which interestingly seem to break in different ways at the moment

Ed
image
Zangief
image

@jmarkman
Copy link
Author

Urien and R.Mika as well! 😳

Urien
firefox_2021-03-20_12-39-37

Mika
image

Ed, Gief, and Mika appear easy enough to fix with a little logic tweaking, Urien might need a little more time.

jmarkman added 11 commits March 20, 2021 13:52
- movesList will now also contain the type of normal it is, i.e.,
this will hold 'held normal' if it's normal that changes
when the button is held

- applying some logic changes to skipFormattingMove to function in line with the changes to the frame data

- menat was breaking the implementation,
i had to create a function for encapsulating the input extraction
- implement 'default to official name' logic for shorthand rename
- add edge case catching for Rashid to skipFormattingMove
- add edge case catching for Young Zeku and Menat to formatMoveName
- logic in index.ts is getting too dense, filled with edge cases
- created MoveFormatter class to hold all rules
and perform shorthand formatting per rule
- created BaseFormatRule class and example derived class
- added the rule for young zeku
- update logic for breaking out of format function in base class
- add rule to MoveFormatter
@jmarkman
Copy link
Author

As we last discussed in Discord, I've tweaked my approach to the shorthand code in order to cleanly and clearly support shorthand normals for SF5, SF4, and Third Strike.

@D4RKONION D4RKONION self-requested a review April 25, 2022 11:02
Copy link
Owner

@D4RKONION D4RKONION left a comment

Choose a reason for hiding this comment

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

Sorry, I was obviously checking out the wrong commit. Still there are some issues that need sorting.

Normalise file names and correctly capitalise imports
image

Neutral Jump HK showing as undefined (Chun Li, E.Honda etc.> Common > Shorthand)
image

GGST broken, shorthand should be ignored for GGST
image

Please use cl rather than c for close
image

jmarkman added 20 commits April 25, 2022 10:11
- replace if-else chain with switch statement
- wrap lodash._mapKeys in function
- add empty function as placeholder for shorthand renaming
- apply vs code "format document"
- add "shorthand" option to naming types in settings
d4rk and i discussed the actual implementation of the
updated function into the app and his vision was to have
an extra entirely separate setting for longform and
shorthand notation, so i've adjusted the function a bit
to prepare for him adding a redux store for that setting
and some various other changes to support it
- 'change displayState' to 'dataDisplayState'
- ensured that input name types won't be converted to shorthand
- remove ternary operator in return statement in helpCreateFrameDataJSON
- fix bug where common inputs with shorthand were
leaving specials "untranslated" to common input
- refactor formatMoveName to work with moveName
property instead of numCmd property due to consistency
changes in the way numeric inputs were represented
- movesList will now also contain the type of normal it is, i.e.,
this will hold 'held normal' if it's normal that changes
when the button is held

- applying some logic changes to skipFormattingMove to function in line with the changes to the frame data

- menat was breaking the implementation,
i had to create a function for encapsulating the input extraction
- implement 'default to official name' logic for shorthand rename
- add edge case catching for Rashid to skipFormattingMove
- add edge case catching for Young Zeku and Menat to formatMoveName
- logic in index.ts is getting too dense, filled with edge cases
- created MoveFormatter class to hold all rules
and perform shorthand formatting per rule
- created BaseFormatRule class and example derived class
- added the rule for young zeku
- update logic for breaking out of format function in base class
- add rule to MoveFormatter
- Simplified if/else chain in renameFrameDataToShorthand
- Update setPlayer and helpCreateFrameDataJSON params
so we can check which game is currently active
- Standardize capitalization in format rule classes
- Update stance to abbreviation map
@jmarkman
Copy link
Author

jmarkman commented May 8, 2022

Had to wrestle with some git history (fingers crossed), but the last few issues should be squared away.

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.

Write a function to swap out long commands for short ones
2 participants