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

Issue with migration from deprecating exact_time_int #1218

Closed
BoomShotKapow opened this issue Aug 27, 2024 · 9 comments
Closed

Issue with migration from deprecating exact_time_int #1218

BoomShotKapow opened this issue Aug 27, 2024 · 9 comments

Comments

@BoomShotKapow
Copy link
Contributor

BoomShotKapow commented Aug 27, 2024

After the deprecation of exact_time_int, the migration case DeprecateExactTimeInt will reset the player's time to the last value of exact_time_int in the database. This value is not updated anymore in the newer versions and it's fine for all new records that have it assigned to 0. But, it's a problem for the existing records before the deprecation, because if the record is improved on after the deprecration, the exact_time_int value isn't updated anymore.

FormatEx(query, sizeof(query), "UPDATE %splayertimes SET time = (1.0 + (exact_time_int & 0x7FFFFF) * pow(2.0, -23)) * POWER(2.0, (exact_time_int & 0x7f800000) / 0x800000 - 127) WHERE exact_time_int != 0;", gS_SQLPrefix);

That line is problematic due to this deprecation, so if you don't backup your database before that's executed (assuming you have records in your database from before the deprecation), then they'll all be set back to the old time and will not reflect the actual record time that's in a replay for example.

@BoomShotKapow BoomShotKapow changed the title Issue with deprecating exact_time_int Issue with migration from deprecating exact_time_int Aug 28, 2024
@rtldg
Copy link
Collaborator

rtldg commented Aug 28, 2024

Hmm. I was originally going to remove the exact_time_int column and then I didn't in case there were migration troubles. And then I didn't account for this. Probably can & should remove the column now.

@rtldg
Copy link
Collaborator

rtldg commented Sep 5, 2024

I think SET time = MIN(time, <exact_time_int thing here>) would be right for the migration so I'll test

@rtldg
Copy link
Collaborator

rtldg commented Sep 23, 2024

I think SET time = MIN(time, <exact_time_int thing here>) would be right for the migration so I'll test

That would work for sqlite but not for MariaDB(/MySQL probably).
Probably have to remove that mysql specific code and do the min() sourcepawn side.

@Nairdaa
Copy link
Contributor

Nairdaa commented Sep 29, 2024

SQLite doesn’t support POWER(), handle the conversion from exact_time_int in SourcePawn instead. You could fetch the records needing updates, calculate the new times on the server side, and then update the database in a transaction batch.

Perform the time update using a safer method. Instead of directly overwriting the time field with calculations from exact_time_int, use MIN(time, calculated_value) to ensure times are not accidentally increased. This ensures that you only update times when the calculated time from exact_time_int is smaller.

FormatEx(query, sizeof(query), "UPDATE %splayertimes SET time = LEAST(time, (1.0 + (exact_time_int & 0x7FFFFF) * pow(2.0, -23)) * POWER(2.0, (exact_time_int & 0x7f800000) / 0x800000 - 127)) WHERE exact_time_int != 0;", gS_SQLPrefix);

Fetch the "id" and "exact_time_int" values where "exact_time_int != 0", perform conversion logic, bla bla you know what to do, then
FormatEx(query, sizeof(query), "ALTER TABLE %splayertimes DROP COLUMN exact_time_int;", gS_SQLPrefix);
and it should be voila.

@rtldg
Copy link
Collaborator

rtldg commented Sep 29, 2024

SQLite doesn’t support POWER()

Yes it does and the timer isn't using POW/POWER there for sqlite anyway.

handle the conversion from exact_time_int in SourcePawn instead. You could fetch the records needing updates, calculate the new times on the server side, and then update the database in a transaction batch.

that's literally what it already does and what said I would make mysql/mariadb do.

Did you just ChatGPT this reply?

@Nairdaa
Copy link
Contributor

Nairdaa commented Sep 29, 2024

No, I took a look at line 536 of what he linked. Would chat GPT fix stuff like that anyway?

@Nairdaa
Copy link
Contributor

Nairdaa commented Sep 29, 2024

FormatEx(query, sizeof(query), "SELECT id, exact_time_int FROM %splayertimes WHERE exact_time_int != 0;", gS_SQLPrefix);

I'm an idiot and misread. Apparently my "an idiot sandwich" steam nickname is correct.

@Nairdaa
Copy link
Contributor

Nairdaa commented Sep 29, 2024

Note to self: Never touch stuff like this at 4am. The idea was good, but I misread.

@rtldg
Copy link
Collaborator

rtldg commented Oct 2, 2024

This should be fixed now with e41ba9f for anyone who hits this.

@rtldg rtldg closed this as completed Oct 2, 2024
bhopppp added a commit to bhopppp/Shavit-Surf-Timer that referenced this issue Oct 4, 2024
[rename FillBoxMinMax to BoxPointsToMinsMaxs
](shavitush@f96a574)

[probably fix exact_time_int deprecation issue (shavitush#1218)
](shavitush@e41ba9f)

[fix bad casts that broke Custom Airaccelerate & Custom Speed Limit zones from](shavitush@ee0fe65)
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

No branches or pull requests

3 participants