Skip to content

Issue #431 Add force retire season - #455

Merged
dumbmatter merged 12 commits into
zengm-games:masterfrom
snzow:master
Aug 17, 2023
Merged

Issue #431 Add force retire season#455
dumbmatter merged 12 commits into
zengm-games:masterfrom
snzow:master

Conversation

@snzow

@snzow snzow commented Aug 15, 2023

Copy link
Copy Markdown
Contributor

@snzow snzow mentioned this pull request Aug 15, 2023
Comment thread src/worker/core/player/shouldRetire.ts Outdated
Comment on lines +12 to +13
const age = g.get("season") -p.born.year;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Don't worry about it for this PR, but it's weird to see formatting like this because Prettier should be running every commit...

Comment thread src/worker/core/player/shouldRetire.ts Outdated
return true;
}

if (p.stats.length >= forceRetireSeason) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This doesn't work correctly.

p.stats is not 1-to-1 with seasons. You get an extra entry if you're traded mid season, and you get an extra entry if you make the playoffs. Easy fix to that would be to count the unique seasons in p.stats

Also, in a new league p.stats is empty. So like for a league in 2023, it generates players from draft classes back to 2019... but then this condition won't trigger for any of them until possibly 4 years in the future. That's probably not what anyone wants.

We could make it based on draft year (assume rookie season is 1 year after draft), which would mostly work. But if people use this to simulate college, they would ideally want the ability to have a red shirt year. So if there is at least one year with 0 games played, it could give that player one extra year before retiring. I think that's probably what most people would want, because IIRC you get only one red shirt year in college, but I could be wrong.

This would also need a special case to handle new leagues, where p.stats is empty. You wouldn't want to assume that every single player had a red shirt year. So you'd probably only want to grant the red shirt year if a season is greater than or equal to g.get("startingSeason").

Does that make sense? It does get a little complicated, I could be missing something.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I originally did it based on draft year but it absolutely massacred the real rosters, which maybe is ok just kind of up to you I guess. I did some manual testing with p.stats and trading mid season and it seemed as though it didn't count it twice but I guess it was some sort of other weird thing. I kind of erred on the side of retiring them after X years of actual playing in the simulation vs X years of theoretical years in the game world but maybe its better to do it that way.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ill do it based on draft year with the red shirt clause for now

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah I think doing it on draft year makes the most sense. It will result in real players leagues seeing a ton of retirements, but I'm not sure anyone would actually want to use this with real players. But people would want to simulate a high school or college league.

@snzow

snzow commented Aug 16, 2023

Copy link
Copy Markdown
Contributor Author

Ok it has been updated to use draft year and give a redshirt based on the number of unique seasons. I tried to come up with a clever way to count unique seasons to avoid iterating through them all but wasn't able to find one.

Comment thread src/worker/core/player/shouldRetire.ts Outdated
Comment thread src/worker/core/player/shouldRetire.ts Outdated
Comment thread src/worker/core/player/shouldRetire.ts Outdated
@snzow

snzow commented Aug 17, 2023

Copy link
Copy Markdown
Contributor Author

ok should hopefully meet all requirements now

@snzow
snzow requested a review from dumbmatter August 17, 2023 02:22
@dumbmatter

Copy link
Copy Markdown
Member

I realized that in my code I forgot to do what I mentioned previously about startingSeason (don't count seasons before startingSeason as potential redshirt seasons, otherwise everyone would get a redshirt season in a new league). So I added that and renamed the variable to forceRetireSeasons because I like that name better :)

I will merge it soon, and then it'll be in the next update - not sure exactly when, but pretty soon.

If you'd like to test the final version, please do, and please let me know if you notice any problems!

And thank you! If you're interested in doing other stuff, feel free to talk to me about ideas.

@dumbmatter
dumbmatter merged commit 788240c into zengm-games:master Aug 17, 2023
@astrxcy

astrxcy commented Aug 18, 2023

Copy link
Copy Markdown

wowee when you think updates coming out

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.

3 participants