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

Add SQLite migration to fix mapzones having NULL id for all rows #1177

Closed jedso closed 1 year ago

jedso commented 1 year ago

SQLite uses ROWID to uniquely identify each row in a table, and a column with type INTEGER PRIMARY KEY becomes an alias for that ROWID value. The CREATE TABLE statement for mapzones since the beginning (1.1b) has been id INT AUTO_INCREMENT, but the column has to be of INTEGER type for it to act as a ROWID alias (not INT). Because id is therefore acting as a ordinary integer table column, all id rows in mapzones have been NULL in SQLite DBs since the plugin was released.

This migration fixes that and gives each map zone a unique identifier for SQLite DBs.

jedso commented 1 year ago

Just had a thought that SQLite DBs created prior to this PR with an empty mapzones table will always return 0 for SELECT EXISTS(SELECT 1 FROM %smapzones WHERE id IS NULL); regardless of whether the table is using id INT AUTO_INCREMENT, so it's probably better to use the SELECT sql FROM sqlite_master + StrContains approach from #1176.

rtldg commented 1 year ago

I'm not a fan of doing the char SQLiteMapzonesQuery[1024]; global variable thing but that's probably the easiest way that won't require changing much else like wrapping things in functions or whatever. Also not that it really matters, but I think it's funny to realize that the bool gB_MigrationsApplied[255]; uses the same amount of memory since bools are cell sized while chars are byte-sized but packed into cell-sized divisible memory.

rtldg commented 1 year ago

I resolved the conflicts on the branch since I just merged the other fk pr. So note to self: this migration number is (now) 29.