shavitush / bhoptimer

A bunnyhop timer plugin for Counter-Strike: Source, Counter-Strike: Global Offensive and Team Fortress 2.
https://timer.shav.it
GNU General Public License v3.0
228 stars 93 forks source link

Normalize and store PB stage times #1181

Open jedso opened 1 year ago

jedso commented 1 year ago

Key changes:

A few minor fixes/changes too:

This PR doesn’t address the implementation of !wrcp, which would need to be a separate table because individual stage time records won’t necessarily be tied to an existing playertime.

The actual migration uses a new stagetimes table with playertimes_id {PK, FK}, stage {PK} and time columns. Technically the stage number column is redundant, and a mapzones id could be referenced to get the stage number from the data column. I opened #1177 to accommodate an implementation that does this, and don’t mind updating the schema/logic if you think it’s a good idea. Would have the benefit of stage times always being associated with the correct stage if a stage zone’s stage number is updated, as well as stage times being deleted at the same time the referenced zone is deleted.

Tested the updated queries in shavit-wr with SQLite and MySQL and both should be working fine. There’s definitely room for cleaner MySQL-specific queries though, as I was focused more on SQLite compatibility. Needs support for Postgres.

Should note that any orphaned records in stagetimeswr are deleted during the migration so you may want to encourage database backups before installation just in case people are relying on WR stage splits that do not belong to an existing playertime.

rtldg commented 1 year ago

I'll go through and check this after the latest advent of code

rtldg commented 1 year ago

Also, what are your thoughts on using the same kind of "more precise" floats for the stage times, as playertimes? AKA, what exact_time_int was replaced with: UPDATE table SET time = %.9f & SELECT %s with gI_Driver == Driver_mysql ? "REPLACE(FORMAT(time, 9), ',', '')" : "printf(\"%.9f\", time)". It's kind of gross to do but it helps keep the same f32 encoding from sp -> db -> sp

jedso commented 1 year ago

Also, what are your thoughts on using the same kind of "more precise" floats for the stage times, as playertimes?

Ah, I forgot about this. Both issues should now be addressed :+1:

Nairdaa commented 1 year ago

Will this be merged into the timer?

rtldg commented 1 year ago

Will this be merged into the timer?

hopefully later today when I test it

jedso commented 1 year ago

Decided to reset last stage to 0 during StartTouchPost instead of being called every tick in TouchPost. Reason being that if you had a start zone intersecting with a stage zone (e.g. stage 1), last stage would never be set to 1 since it gets immediately overridden. Should work fine now for that scenario.

Nairdaa commented 1 year ago

"had a start zone intersecting with a stage zone (e.g. stage 1), "

Isn't startzone stage one by default? As in, if you make a stage mid map and run into it, it should show "Stage: 2", while leaving the startzone should be "Stage: 1". Correct me if i'm wrong.

rtldg commented 1 year ago

"had a start zone intersecting with a stage zone (e.g. stage 1), "

Isn't startzone stage one by default? As in, if you make a stage mid map and run into it, it should show "Stage: 2", while leaving the startzone should be "Stage: 1". Correct me if i'm wrong.

IMO it's never really been cemented for stages in bhoptimer since there's only "Stage" zones instead of both "Stage start" & "Stage end" zones.

With how it is, I think treating "Stage" zones as "Stage end" zones is a good way to do it. AKA you'd be on stage 1 by default and then you'd hook the teleporter to stage 2 with as the stage 1 zone. And then the stage time when you hit the tele would count for the stage 1 time. Idk if it's the best but it seems okay...

Maybe the expected way to use them should be clarified somewhere or a "Stage start"/"Stage end" zone could be added to make it more explicit.

jedso commented 1 year ago

With how it is, I think treating "Stage" zones as "Stage end" zones is a good way to do it. AKA you'd be on stage 1 by default and then you'd hook the teleporter to stage 2 with as the stage 1 zone. And then the stage time when you hit the tele would count for the stage 1 time.

If !wrcp is going to be implemented in the future, then this isn't a good approach because you need a stage start zone after the tele to know when to start timing an individual stage run time. I took a look at how Momentum Mod and a random surf server handles stages, and it looks like a single stage zone acts as both a "Stage start"/"Stage end" zone simultaneously.

E.g. you would place a "Stage 2" zone after the teleport, and for a full track run touching it would mark the end of how long it took you to complete Stage 1. Jumping/leaving the Stage 2 zone would then begin the individual stage timer, so it also acts as a stage start zone. FWIW, in surf it seems like the individual stage timer only starts if there is no bhop pre-speeding in the stage start zone.

On that point, the other issue to work out is exactly what a stage time is measuring. Currently, IMO, the ZoneStageEnter message You have reached stage N with a time of XX:XX. implies that a stage time is recorded when somebody touches the next stage zone. E.g. as a diagram: touch-zone-v4

The other way of measuring a stage is the time taken to complete each individual stage. This is obviously what you would want for !wrcp, but it can also apply to a full track run if logic is added to support a single stage zone acting as both a "Stage start"/"Stage end" zone: each-stage-v1

Rewording ZoneStageEnter from "reached" → "completed" makes more sense in this case.

The second approach is exactly how Momentum Mod handles stage splits and stage zones currently. This includes showing the stage 3 split as a duplicate of the final playertimes time (57.25), which in practice I don't think you'd actually want to store in the stagetimes table. Timing stage 3 start zone → track end zone is really only useful for individual !wrcp stage times, otherwise it's just redundant data.

As it exists right now, the timer can do the first approach without any further changes. The second approach requires additional logic but would also help with a future !wrcp implementation. Let me know any thoughts.

Nairdaa commented 2 days ago

Bump :P