Infinite antag tags

[marlyn-x86]

This will change how we store our antag tags from a single integer to a list, allowing us to have as many antagonist/role types as we wish.

This is a port of @RemieRichards's HoG infinite antag refactor tgstation/tgstation#11803. Due to differences in our codebases, this will be a bit more complicated than a "ctrl-c +ctrl-v", but I figure it'll be handy, let us keep space ninja viable, and give us more than 16 types of special roles.

Before merging:
BACK UP THE DATABASE IN CASE SOMETHING GOES WRONG

New role tags:

• Borer
• Sentient animal
• Slaughter Demon
• Positronic Brain
• Guardian

[tigercat2000]

😻

[Chiquiii]

will this mean multiple game modes at one?

[Fox-McCloud]

@orelbon

No; this is mostly for coders--players won't notice a single difference.

Basically for gamemodes/antag types, there's flags to track what a player has selected. Unfortunately, there's so many game modes now, we have run out of bitflags...meaning we have to start cannibalizing old flags or implement a new system( lik this one) to get around that.

This does just that, meaning we can now have 5,000,000,000 game modes, if we want, without a problem.

[marlyn-x86]

This is the sort of thing where having a functional DB is important when testing, but I don't, and I haven't found a good resource on how to do so yet. Any pointers?

[Fox-McCloud]

@tigercat2000

[marlyn-x86]

Mmm, -tg-station uses BYOND save files, which have native handling for BYOND lists, while SQL does not. Does SQL support any form of list fields?

And how would I check whether entries in the SQL DB are of one format or another, since I'll need to switch the existing entries' format from one to another?

[tigercat2000]

@Crazylemon64 SQL doesn't support BYOND's idea of lists natively, no. The current standard is to use list2params in order to turn a list into a text string of parameters, then upload it to SQL after sanitizing it.

All of the SQL handling is done in this file, via MySQL queries that the server runs upon saving and loading characters.

I would recommend trying to follow -tg-'s wiki article on setting up a SQL database, as it is quite helpful, except, instead of using SQL/tgstation.schema, you would use SQL/paradise_schema.sql.

As for checking entries in the SQL Database, you would require a basic knowledge of how SQL Queries work, plus, if you are refering to the server database, only the headmins and maintainers have access to it. Your best bet would be modifying the schema file, and getting rid of the be_special column entirely, and replacing it with something like antag_prefs mediumtext NOT NULL.
From there, you have to modify how SQL is saved and loaded- specifically, this line should instead look something like antag_prefs='[sql_sanitize_text(list2params(whatever_the_antag_prefs_list_is))]'

Actually, sql_sanitize_text is probably unnecessary here- on one condition; You have to absolutely make sure that every antag name does not have any of the following characters; ' , / ; \improper \proper

and this line should instead look like antag_prefs (you get the idea)
This line should be params2list instead of text2num, and this would be gone, I guess

[marlyn-x86]

@tigercat2000 I'll need to insert the new field at the end of the DB, and leave the be_special field fallow, right? Otherwise I'd expect it to mess up the SELECT * FROM query, since that wouldn't account for the row being gone, right? Or is there some way to swap out rows in place?

[marlyn-x86]

Although travis does this like 18x faster than my machine can, I probably shouldn't bloat my commit count with repeated error-filled commits

[tigercat2000]

@Crazylemon64 Ah, right, forgot to respond to this-
You might have a problem if you were updating the load_character proc, however, be_special is handled at load_preferences, which does not use a SELECT * query- it's only selecting certain columns, so it doesn't matter in which order the columns are in, it will produce the same column order in the output

There are, also, ways to insert a column in a table mid-line, but it's annoyingly complex queries.

[marlyn-x86]

But I will need to update the load_character proc too, right? Or am I overlooking something...

[tigercat2000]

Huh.

You actually need to remove the be_special completely from load_character(), that shouldn't be there.
I have no idea why it's duplicated across two tables.

[marlyn-x86]

I think it's so people can have characters with different antag preferences - this one is a cultist, another works for the syndicate, something like that

Or does this not work at all and whoever originally wrote that gibbed themself with a faulty line of code?

[tigercat2000]

@Crazylemon64 Except the preference is just overriden by whichever loads last, the character or the player prefs; And everything else in the game prefs panel is not per-character

[marlyn-x86]

Alright, ripping that bit out, then. I'll see about getting something potentially workable shortly.

[marlyn-x86]

@tigercat2000 I'm having trouble starting up Apache. I lack control over the ports in my network, will that interfere with setting up XAMPP for the SQL database?

EDIT: I think I got something working, I had XAMPP installed in a system directory, where it should have been in a user directory

[marlyn-x86]

Database set up, now to see if things load swimmingly...

They do! Now to see if I can't set up some conversion procs from our old tag system and finish up this PR...

[marlyn-x86]

I assume this sql_sanitize guard is what's torpedoing the entry adding, and causing everything to be concatenated to a single entry. I'll create a proc to sql_sanitize the entries of a list, and use it here, so that we don't leave any opening for a "Bobby tables" attack

[tigercat2000]

AAaahh yes, sql_sanitize strips semicolons, because semicolons are bad in SQL queries
However, as you can't have a custom be_role outside of pressing buttons, you can just remove this entirely

[marlyn-x86]

The buttons work as url links through byond://, right? Couldn't someone use wireshark to see what they're sending, tweak the format, and ask for the role "; DROP TABLE players"? Anyways, I've added a proc to sql sanitize each entry of a list, so we shouldn't be at risk of this happening anymore.

[tigercat2000]

That's why you should always be careful with how Topic() code is written, but I don't think it would be a problem in this specific case.

[marlyn-x86]

There's a clause I've added in preferences.dm to watch for anyone trying to pass in a nasty href, but I can't pull it up because I adjusted the indentation in that file; is that enough to shield us from a potential exploit?

[marlyn-x86]

Alright, this all works now, now to finally do a conversion proc from old format to new

[marlyn-x86]

Alright, I think I'm pretty much done, just need to wait on @ZomgPonies's #2896, though. What will the strategy be for updating the DB for this new format?

[Fox-McCloud]

Alright, I think I'm pretty much done, just need to wait on @ZomgPonies's #2896, though

Yours comes first, as we're pretty much deadlocked for being able to add anything remotely related to antag flags right now, and that PR hasn't been touched in a long time.

[tigercat2000]

did someone say memes

[Fox-McCloud]

I actually had no problem with the meme, @Crazylemon64; I just wanted it to be accurate =p

[marlyn-x86]

Well, now that THAT matter's taken care of, I think I want to get one last thing scored away, so that people don't rejoin one day and have to reset their role preferences - and of course I need to fully verify that nothing runtimes from switching the antag tags from numbers to strings. However, for now I have to leave, so once I come back and polish that off, I'll post one last update.

[marlyn-x86]

Well, looking at the administration-side, I probably want to update the job-ban interface so that the new tags show up, lest a jerk show up and the admins be forced to resort to rooting through proc calls to deliver justice - hold on

[marlyn-x86]

Ah, you know what, updating the ui can wait - what matters is grouping the new antagonistic roles under the antag list so that a syndicate ban will conclusively put a hold to the shenanigans. Once that's through, merge away. (After proper review of course)

[marlyn-x86]

Screw it, I'm not finding a list that corresponds. One last commit and I'm done.

[Fox-McCloud]

Also, you'll want to make sure that all the news ones show up in the player preferences.

And that no scrolling/scroll bars appear from the increased size of the new prefs.

[marlyn-x86]

Fortunately, the game preferences automatically handles additional game roles, so we're golden there, and no, it doesn't require scrolling.

[marlyn-x86]

Alright look through and check if I made yog-sothoth come out of the depths of our database and eat all our precious characters or not, though it seems like it holds fast.

Merge Ready.

[Fox-McCloud]

@tigercat2000

@TheDZD

[Spacemanspark]

Memes.

[TheDZD]

Looks alright to me, but this shouldn't be merged until we can get a patch immediately afterwards, since this requires SQL changes.

[Fox-McCloud]

ROLE_BORER

[ZomgPonies]

@Crazylemon64 Give me clear instructions for the prod DB. Do I simply add your new column, or do I also remove be_special?

[marlyn-x86]

@ZomgPonies You can leave be_special in place for now, until most players get their format transferred over to the list system; Once you've had that in place for "long enough", you can get rid of both the format update proc in preferences_sql.dm, and the be_special column, without any problems.

[tigercat2000]

@Crazylemon64 bit late, already did a more destructive method of delete characters.be_special, alter player.be_special, rename to be_role and change type to mediumtext

[marlyn-x86]

Mmm, alright. You can rip out the conversion proc, then.