R2Northstar / NorthstarMasterServer

Master server for Northstar
MIT License
92 stars 33 forks source link

Implementing pdiff (mod persistence) #59

Closed ASpoonPlaysGames closed 2 years ago

ASpoonPlaysGames commented 2 years ago

Basic stuff to do:

More advanced (and painful) stuff to do:

Edit: must be merged alongside NorthstarLauncher/148

ASpoonPlaysGames commented 2 years ago

Ok so, basic reading and writing is functional (kinda), clients dont send mod data when they auth with themselves, so I can't send them any pdiff data

Also a good chunk of the time the client with fail auth with the server, with an error in parsing the json file. I don't know why this is happening, I have checked and the object seems perfectly fine before it gets converted to a buffer and sent from the masterserver, potentially the issue is with PdataJsonToBuffer, or some northstar launcher code

I also need to filter the regular pdata out from the pdiff data that gets sent by the gameserver to write to the database, currently I am ignoring it, and only updating the pdiff data

ASpoonPlaysGames commented 2 years ago

Current progress: reading and writing basic pdiff is functional, however I am yet to split the pdiff from the rest of the pdata in a way that conserves the remaining pdata, which could then be written to the pdata baseline.

TLDR: you can do pdiff, but if you are doing pdiff you cant do normal pdata atm

ASpoonPlaysGames commented 2 years ago

pdiff now seems to be functional, for pdiffs that do not add to enums and stuff like that, idk where its going wrong in that regard

basically pdiffs like this are fine

$PROP_START $ENUM_START pDiffTest tdm cp $ENUM_END int isPDiffWorking string{9} testString string{9} testString2

But pdiffs like this break stuff:

$ENUM_ADD gameModes fuck $ENUM_END

$PROP_START $ENUM_START pDiffTest tdm cp $ENUM_END int isPDiffWorking string{9} testString string{9} testString2

ASpoonPlaysGames commented 2 years ago

So basically, this could be considered ready for review? We would then release it in a sort of beta state?

ASpoonPlaysGames commented 2 years ago

Oh one more thing to note, when I create a string the game doesnt seem to like to write to the last char, always leaves it blank, idk if its like that with normal pdef or not though

ASpoonPlaysGames commented 2 years ago

This is now ready for testing I think, it does require the linked northstar launcher PR though

GeckoEidechse commented 2 years ago

Skimming through it, I haven't spotted an explicit way to limit pdiff size per player/mod. We should definitely limit it per player and per mod to ensure both that players do not fill up MS with garbled data and mod authors are not tempted to create large pdiffs. We can always increase limits later ofc but decreasing is a bit harder :P

ASpoonPlaysGames commented 2 years ago

Yeah so there isn't really a way to limit pdiff size per player/mod, because of how pdiffs interact with each other and vanilla pdata, the size of the data stored in the MS is variable.

for example: if i set my primary weapon in my loadout to mp_weapon_peacekraber, it would get saved in the pdiff of the peacekraber's mod, however when i change it back to what it was before, it is no longer saved there, and therefore the pdiff data shrinks

ASpoonPlaysGames commented 2 years ago

I could enforce a pdiff size limit but tbh I'm not sure that it's a particularly good idea, because it might begin to auth fail people or something at seemingly random times, as the pdiff data size might change based on loadout selections or even other installed mods

GeckoEidechse commented 2 years ago

I could enforce a pdiff size limit but tbh I'm not sure that it's a particularly good idea, because it might begin to auth fail people or something at seemingly random times, as the pdiff data size might change based on loadout selections or even other installed mods

Yeah we would need to have proper error handling where a too large pdiff would send an error message/code back to client that then parses it and shows the player the appropriate error :I

ASpoonPlaysGames commented 2 years ago

a too large pdiff would send an error message/code back to client

annoyingly i dont think a client would ever actually get that message, since pdata is saved when the map changes or the player leaves, and it is also only ever networked to clients through the server, we never send pdata to clients directly

ASpoonPlaysGames commented 2 years ago

I might change the way that we get a hash for the pdiffs, as the current method results in "lost" data in the DB if the pdiff file ever changes, so if we ever use pdiff in things like Northstar.Custom and update the pdiff file it would be less than ideal. Probably going to look into making sure that the pdata doesn't get malformed when loading pdiffs after a pdiff file has been modified, and then probably hashing the mod name instead of the pdiff file

GeckoEidechse commented 2 years ago

This is still WIP, right?

ASpoonPlaysGames commented 2 years ago

This is still WIP, right?

Unfortunately, yeah, Respawn added an arbitrary limit that kinda ruins pdiff, so it's probably going to need a decent size rework

pg9182 commented 2 years ago

Closing for now as discussed on Discord.