Tidy up the way experience/character levels are handled

[ephphatha]

Prep work for loading experience thresholds from an external source.

The getter/setter really blows out the diff but this was mainly in response to the code paths that unpacked player data and only set _pLevel, relying on _pNextExper getting set in InitPlayer().

[obligaron]

Perhaps we should go even further

• remove _pNextExper
• change AddPlrExperience to a member function of Player
• change _pExperience to private (like _pLevel)

[ephphatha]

change _pExperience to private (like _pLevel)

I haven't implemented this as when I went to do the refactor I realised there were more assignments to _pExperience that didn't want to change the character level than there were which did. It didn't really add any benefit. For reference the writes to experience:

• When using a glowing shrine (can lower the players experience below the previous level's threshold, but should not lower the character level).
• When clamping the players experience in ValidatePlayer (sets it to the threshold for the next level only if it somehow exceeded the threshold as a sanity check)
• When saving/loading or packing/unpacking players (should not alter the character level in case the player has used a glowing shrine)
• When using the debug level up cheat
• When adding experience to the player from a medicant or sparkling shrine
• When killing a monster

The last two uses went through AddPlrExperience anyway, the only potential improvement would've been for the debug command.

[ephphatha]

Oh, just realised why the demo is failing even after a rebase. The demo save file has _pMaxLevel set to 2, _pLevel is 3. I'm assuming the player's maxLevel stat was reset when the player struct was packed and unpacked (when starting the game?), then the player leveled up twice.

A dodgy "fix" would be to set _pMaxLevel to _pLevel - 1 when saving but we can probably just skip it without harm as well.

[obligaron]

change _pExperience to private (like _pLevel)

I haven't implemented this as when I went to do the refactor I realised there were more assignments to _pExperience that didn't want to change the character level than there were which did. It didn't really add any benefit. For reference the writes to experience:

* When using a glowing shrine (can lower the players experience below the previous level's threshold, but should not lower the character level).

This could be done with calling AddPlrExperience with a negative value?

* When using the debug level up cheat

This could be done with calling AddPlrExperience normally? 🤔

* When clamping the players experience in ValidatePlayer (sets it to the threshold for the next level only if it somehow exceeded the threshold as a sanity check)

I'm not sure if this check is needed anymore. Do we want to prevent in-memory modifications?

* When saving/loading or packing/unpacking players (should not alter the character level in case the player has used a glowing shrine)

This could also be combined in a loadPlayerStats function or something like that, this would also ensure we don't get out of-bounds.

Context:
My comment is only a minor suggestion and not something that should hold of this PR.
The PR is fine as it is. Thanks for the done additions. 🙂

[obligaron]

lgtm, only some minor nits/questions left. 🙂

[ephphatha]

change _pExperience to private (like _pLevel)

I haven't implemented this as when I went to do the refactor I realised there were more assignments to _pExperience that didn't want to change the character level than there were which did. It didn't really add any benefit. For reference the writes to experience:

* When using a glowing shrine (can lower the players experience below the previous level's threshold, but should not lower the character level).

This could be done with calling AddPlrExperience with a negative value?

We could make AddPlrExperience handle negative values but it would still need care to avoid the scaling that gets applied in multiplayer games. Calling an Add function with a negative value also seems a bit unusual to me, this would probably be best as a new Player::takeExperience function. I reckon it would be worthwhile but don't want to do it in this PR :D

* When using the debug level up cheat

This could be done with calling AddPlrExperience normally? 🤔

Need to bypass the power levelling cap, this would need a debug-only flag for only that call site.

* When clamping the players experience in ValidatePlayer (sets it to the threshold for the next level only if it somehow exceeded the threshold as a sanity check)

I'm not sure if this check is needed anymore. Do we want to prevent in-memory modifications?

Probably not, I ended up removing the clevel check in IsNetPlayerValid because it was no longer needed for that purpose. I couldn't see any other reason for that behaviour in ValidatePlayer though so figured it was safer to leave it there.

[obligaron]

lgtm 🙂

[ephphatha]

This PR can be closed if we don't want to include this change in 1.5.1, #6399 includes all these changes and is already rebased on development.