core does not validate that playergroups sql table contains valid jobs/gangs

[solareon]

Summary

playergroups not validated before being added to playerdata

Reproduction

Add gang/job to shared luas. Add gang/job to player via /addjob or /addgang. Remove gang/job from shared lua. The gang/job remains present in the PlayerData.jobs table.

Expected behavior

Invalid jobs/gangs are removed from the PlayerData.jobs table either on server startup or player load and an error message is thrown to advise the user to remove this bad data from the sql table

Actual behavior

gang/job remains present in the PlayerData.jobs table.

Additional context

No response

Current Version

v1.17.2

Custom Resources

any multijob utilizing core exports.

[Manason]

The solution should be done on player load and should remove using the exports, which will also take care of updating the database. No warning needed.

[Manason]

Although because jobs and gangs can be added at runtime, there is some risk of deleting player data just because a dependent script couldn't properly start. So there's also an argument not to modify the database.

[solareon]

Although because jobs and gangs can be added at runtime, there is some risk of deleting player data just because a dependent script couldn't properly start. So there's also an argument not to modify the database.

Was going to say just randomly nuking a players db table cause of something being busted in the lua might be a bit harsh. I think warning the user on the console and then not loading those jobs into the PlayerData.jobs would be probably the best compromise between not breaking too much stuff and not punishing a user for mistakes

[Manason]

Although because jobs and gangs can be added at runtime, there is some risk of deleting player data just because a dependent script couldn't properly start. So there's also an argument not to modify the database.

Was going to say just randomly nuking a players db table cause of something being busted in the lua might be a bit harsh. I think warning the user on the console and then not loading those jobs into the PlayerData.jobs would be probably the best compromise between not breaking too much stuff and not punishing a user for mistakes

I agree

[Manason]

On second thought @solareon The more common use case would be removing jobs/gangs, so shouldn't we optimize for that to reduce overall manual effort? I.e. it would be easier to recover from a mistake if accidentally deleting database stuff, versus require players to manually delete database stuff whenever they remove a job/gang

[solareon]

On second thought @solareon The more common use case would be removing jobs/gangs, so shouldn't we optimize for that to reduce overall manual effort? I.e. it would be easier to recover from a mistake if accidentally deleting database stuff, versus require players to manually delete database stuff whenever they remove a job/gang

So I think we can add another command similar to convertjobs where we clean the playergroups table when ran. This way if a user has old jobs they wish to remove we can do it cleanly for them while also allowing for mistakes

@Manason

[solareon]

PR submitted that should address all these issues