-
Notifications
You must be signed in to change notification settings - Fork 7
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
[Move] Make invalid move lists ReadonlySet
s and update their contents
#874
Conversation
Btw it's best to view the changes with "Hide Whitespace" https://github.com/Despair-Games/poketernity/pull/874/files?diff=unified&w=1 |
Is there a list of which moves are now allowed in the PR desc? Also there should probably be a comment above the invalid move lists that explain there's a custom implementation to allow certain moves.; |
c0e1587
to
3c4cedb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just tested it, and unfortunately, freezing a set doesn't seem to work (e.g., calling this.invalidMoves.delete(moveId)
still deletes the move). This PR will still be useful for providing type checks, though.
3c4cedb
to
4e44930
Compare
Hm, maybe I have to call |
No that didn't work either, wtf?
|
Well I tried a bunch of stuff and wasn't able to make it work like I wanted, oh well... there should still technically be a performance benefit (even if it's very negligible lol) and leaving it doesn't break anything. |
@Tempo-anon Well you can just look in the files changed tab and see the list right there so I didn't put one in the PR description (you may need to disable "whitespace changes" via the link I provided above or by checking this checkbox): |
I updated the PR desc to include the full changes. There should still be a comment near the sets (maybe linking to this PR) to indicate that they are custom sets with extra moves allowed |
What are the changes the user will see?
More potential moves used by Metronome
More potential moves used by mirror move
Why am I making these changes?
Change to
ReadonlySet
s from arrays: These lists should not be modified at runtime.Change to list contents: More fun.
What are the changes from a developer perspective?
CallMoveAttr.invalidMoves
is nowReadonlySet<MoveId>
instead ofMoveId[]
.invalidCopyCatMoves
,invalidMetronomeMoves
,invalidMirrorMoves
,invalidAssistMoves
andinvalidSleepTalkMoves
.invalidMetronomeMoves
and some frominvalidMirrorMoves
because we're not Gamefreak.Checklist
beta
as my base branch