Skip to content

init: Add timeout validation tests and enhance doTimeout logic#498

Open
jdabapo wants to merge 7 commits intozengm-games:masterfrom
jdabapo:chore/fix-improper-timeouts
Open

init: Add timeout validation tests and enhance doTimeout logic#498
jdabapo wants to merge 7 commits intozengm-games:masterfrom
jdabapo:chore/fix-improper-timeouts

Conversation

@jdabapo
Copy link
Copy Markdown
Contributor

@jdabapo jdabapo commented Mar 4, 2026

add test cases for doTimeout and make it so its impossible to call timeouts in scenarios where its not allowed (XP, 2pt conversion, during kickoff, before kickoff (I believe is managed by the awaiting Kickoff variable?)

}

doTimeout(t: TeamNum, toStopClock: boolean) {
doTimeout(t: TeamNum, toStopClock: boolean, playType: teamPlayType) {
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.

Can be slightly more concise by changing it to playType: ReturnType<GameSim["getPlayType"]> then you don't need an extra type.

return new Promise<void>((resolve) => {
server.listen(port, exposeToNetwork ? "0.0.0.0" : "localhost", () => {
console.log(`Local: http://localhost:${port}`);
console.log("Worker console: chrome://inspect/#workers");
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.

Would rather not put this here since it's Chrome specific, other browsers have different ways of accessing it.

Comment on lines +1310 to +1318
if (playType === "kickoff" || playType === "punt") {
return;
}
if (playType === "extraPoint" || playType === "twoPointConversion") {
return;
}
if (this.awaitingKickoff) {
return;
}
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.

I would move this logic out of here, and instead up to line 977. Reasons:

  • When toStopClock is true, we don't care about these constraints, they are superseded by the other logic about isClockRunning etc
  • Weird to have a function doTimeout that doesn't always do a timeout.

Also, for some of these (punt) we probably want "fewer timeouts" rather than "absolutely no timeouts".

If this logic becomes too complex, abstract into its own function. Like shouldCallTimeout which takes down, distance, clock, quarter, score, etc. and returns a boolean (or undefined/"toStopClock"/"other". Cleaner. Because ultimately to do a good job with timeout logic (especially for the clock management ones) it'll need to be a little complex.

if (playType === "extraPoint" || playType === "twoPointConversion") {
return;
}
if (this.awaitingKickoff) {
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.

If this.awaitingKickoff is defined, that means the next play will be "kickoff" or "onsideKick" so just use those playTypes directly in your logic.

If the goal is to detect the first play of a drive, I'm not sure if there is a simple way to do that right now, so you could add some new variable similar to the awaiting* ones.

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