MomTV Run Submission #1085
MomTV Run Submission #1085
Conversation
tsa96
force-pushed
the
feat/0.10-run-submission
branch
from
c96e3ed to
6909af4
Compare
apps/backend/src/app/modules/session/run/run-processor.class.ts
Outdated
Show resolved
Hide resolved
tsa96
force-pushed
the
feat/0.10-run-submission
branch
from
6909af4 to
aabd95d
Compare
| if (header.gamemode !== session.gamemode) | ||
| throw new RunValidationError(ErrorType.BAD_META); | ||
|
|
||
| if (!approxEq(header.tickInterval, Tickrates.get(session.gamemode))) |
There was a problem hiding this comment.
Tick interval should be exactly the official value I would think
There was a problem hiding this comment.
If this is a matter of float32 vs JavaScript's native doubles, Tickrates should be defined as float32.
On a side note I just realized it's called Tickrates but stores tick intervals (the inverse of tickrate). I think we have a bad habit of using the terms interchangeably in game code too.
There was a problem hiding this comment.
Didn't know there was a nice way to do this with native JS until now, but turns out Math.fround is a thing and does what we're after.
My bad wrt the tickrate/intervals thing, definitely something we should be using precisely, have updated the naming.
| const headerRunTime = header.runTime * 1000; | ||
|
|
||
| // When the run was started according to replay file | ||
| const runStart = header.timestamp - headerRunTime; |
There was a problem hiding this comment.
It's not an issue yet but I wanted to note something interesting to keep in mind: some of our movement code improvements involve stopping a movement tick short of completing the full tick, and in the future (probably coinciding with sub-tick timing in general), the timer will reflect these shortened ticks by ticking less than a full tick of time. This means that the end of run true timestamp minus the start of run true timestamp will likely be slightly more than the run duration. For most runs this will likely never matter, though it could amount to maybe even a few minutes difference for a 10 hour run. If needed, maybe we will add timestamps to RunSplits corresponding to the run start and each checkpoint.
| throw new RunValidationError(ErrorType.BAD_TIMESTAMPS); | ||
| } | ||
|
|
||
| if (checkpointsOrdered) { | ||
| for (let i = 1; i < timestamps.length; i++) { | ||
| if (timestamps[i].checkpoint <= timestamps[i - 1].checkpoint) { | ||
| if (timestamps[i].minorNum <= timestamps[i - 1].minorNum) { |
There was a problem hiding this comment.
I think there also needs to be a check for no duplicate minorNum when checkpointsOrdered === false and checkpointsRequired === false
There was a problem hiding this comment.
Duplicate minorNums should never be possible, so I check for them validateSessionTimestamps already. Added a test for the case you give to make sure, which already passes.
| throw new RunValidationError(ErrorType.BAD_TIMESTAMPS); | ||
| } | ||
| // First minorNum is always 1 (a segment start), regardless of track type | ||
| if (timestamps[0].minorNum !== 1) { |
There was a problem hiding this comment.
Is timestamps guaranteed to be ordered at this point, or could it still be out of order depending on network conditions? There's a few places where timestamp elements are accessed according to an assumed order.
There was a problem hiding this comment.
Will refactor as discussed on Discord, but need to do when more awake (next couple days)
| export interface RunSegment { | ||
| subsegments: RunSubsegment[]; | ||
|
|
||
| // Contains an entry for every subsegment the player has reached so far |
There was a problem hiding this comment.
This comment should be in front of subsegments (the original C++ has the same error lol)
| velocityWhenReached: vec3; | ||
| } | ||
|
|
||
| export interface RunStats { |
There was a problem hiding this comment.
(nit) I can see you've just taken the original C++ class names and prepended "Run" to namespace them better, but it makes this one a little misleading since these stats can apply to not just a whole run but sometimes a segment of a run or a single subsegment.
There was a problem hiding this comment.
Sure, can use a RunSplits namespace here.
|
|
||
| const header = readHeader(buffer); | ||
| expect(header).toEqual(expectedHeader); | ||
| expect(header.tickInterval).toBeCloseTo(0.01, 5); |
There was a problem hiding this comment.
Tick interval should be exact (making sure to compare float32 to float32 so no wiggle room is needed, not sure how easy/hard this is in JavaScript).
There was a problem hiding this comment.
Turns out, way easier than I initially though! Using Math.fround(0.01) instead of 0.01 passes exact equality.
|
Addressed a couple of rio's things but haven't organised commits yet or addressed other stuff, will do tomorrow |
tsa96
force-pushed
the
feat/0.10-run-submission
branch
from
aabd95d to
b529ce2
Compare
|
Addressed everything besides the timestamp ordering stuff which I'll do soon |
tsa96
force-pushed
the
feat/0.10-run-submission
branch
3 times, most recently
from
d42eff3 to
9204e11
Compare
Not branding these, too annoying, just doing this for clarity when parsing replay headers
Writer is just for E2E tests
tsa96
force-pushed
the
feat/0.10-run-submission
branch
from
9204e11 to
bc89ee4
Compare
…data Idk what I was doing here, some stupid optimisation to only select certain fields, was making life harder for Panzer. Also switches from "stats" to "splits", which is changing in next few commits.
Forgot to convert this to 1-indexing
We no longer have individual ts files for each model so comments didn't make sense.
tsa96
force-pushed
the
feat/0.10-run-submission
branch
from
bc89ee4 to
77f145d
Compare
This results in named logger instances having two 'context' fields.
tsa96
force-pushed
the
feat/0.10-run-submission
branch
2 times, most recently
from
808b839 to
d4a93b5
Compare
tsa96
force-pushed
the
feat/0.10-run-submission
branch
from
d4a93b5 to
03fc68a
Compare
Reworks run submission to use new replay format including significantly refactoring validation logic. I've tested fairly extensively in-game, all working well.
Keen that this is reviewed by game devs, especially @jason-e. Most important thing is
run-processor.class.tswhich handles validation, and stuff inlibs/formats/replay.Checks
nx run db:create-migration <name>and committed the migration if I've made DB schema changesfeat: Add foo,chore: Update bar, etc...fixuped into my original commits