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

PRNGSeed is now a string #10826

Merged
merged 1 commit into from
Jan 15, 2025
Merged

PRNGSeed is now a string #10826

merged 1 commit into from
Jan 15, 2025

Conversation

Zarel
Copy link
Member

@Zarel Zarel commented Jan 15, 2025

This makes it so we no longer need to ad-hoc convert seeds from strings to arrays when we get them from text protocols like the command line or BattleStream's reseed command.

It also has the side benefit of making inputlogs very slightly smaller.

This makes it so we no longer need to ad-hoc convert seeds from strings
to arrays when we get them from text protocols like the command line or
BattleStream's `reseed` command.

It also has the side benefit of making inputlogs very slightly smaller.
@Zarel Zarel requested a review from AnnikaCodes as a code owner January 15, 2025 07:06
return seed.join(',') as PRNGSeed;
}
static get(prng?: PRNG | PRNGSeed | null) {
return prng && typeof prng !== 'string' && !Array.isArray(prng) ? prng : new PRNG(prng as PRNGSeed);
Copy link
Member

Choose a reason for hiding this comment

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

is the typecast necessary here? doesn't it default to PRNGSeed | null after the first check is done?

Copy link
Member Author

Choose a reason for hiding this comment

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

image

Copy link
Member Author

Choose a reason for hiding this comment

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

The Array.isArray part confuses it. Reasonable because arrays are not PRNGSeeds anymore, but annoying for us and requires a cast.

@Zarel Zarel merged commit fde2b11 into master Jan 15, 2025
1 check passed
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.

2 participants