Adding new UdonSynced fields to a Player Object appears to make saved data read-only / unsyncable / Guideline to add new UdonSynced fields in a VRCEnablePersistence object unclear
tracked
Haï~
Steps to reproduce (TLDR:
bold
):- Create a world that has this UdonBehaviour(OurTestObject.cs):https://gist.github.com/hai-vr/3439f25571fcc4ad4f65107ad46657d8
- Hierarchy example:
- Pool
- PlayerObject -- VRCPlayerObject + VRCEnablePersistence
- OurTestObj -- UdonBehaviour(OurTestObject.cs)
- Canvas 5x5
- Text -- TMP_Text 5x5 Alignment=Center&Bottom Wrapping=Disabled Overflow=Overflow
- In UdonBehaviour(OurTestObj), reference the TMP_Text component
- Upload the world
- Start two real clients
- Make Client A join the world
- Make Client B join Client A's world
- Make Client A rejoin the world
- After Client A finished rejoining the world, Make Client B rejoin the world
- At this stage, timesJoined should equal to 2 for both clients, confirming that Persistence is working for this world.
- In OurTestObject.cs, uncomment line 2. This will add a new UdonSynced bool field initialized to false, and a new line in the debug text.
- Recompile and reupload the world.
- Make Client A create and join a new instance of that world
- In Client A's perspective, notice the following in the debug text over the their own head:
- The dateTime timer is ticking
-
timesJoined is equal to 3
-
testBool has no text displayed (neither true nor false), but testBool is defined as a bool in U#
- (testBool appears to be null)
- Make Client B join Client A's instance
- In Client B's perspective, notice the following in the debug text over the Client A's head:
- The dateTime timer is NOT ticking
-
timesJoined is equal to 2 (instead of 3)
-
testBool has no text displayed (neither true nor false), but testBool is defined as a bool in U#
-
The message "No new data received since ...s" appears, indicating no serialization has occurred.
- In Client A's perspective, notice the following in the debug text over the Client B's head:
-
The observations are identical to Client B's of Client A
- (serialization is failing, so no new data is received)
- (despite the above, data has been restored to persistent values because timesJoined is greater than 1)
- (testBool appears to be restored to null from the perspective of other clients, prior to the failed serializations)
- In Client B, go to the world description and click "Reset User Data", this will make Client B rejoin the world
- In Client B's perspective, notice the following in the debug text over the their own head:
-
testBool is equal to false
- In Client A's perspective, notice the following in the debug text over the Client B's head:
- testBool is equal to false
- The dateTime timer is ticking
- (resetting the User Data has correctly set testBool to its expected value of false, so testBool is correctly supposed to be initialized to a non-null value of false)
- In Client A, rejoin the world.
- In Client B's perspective, notice the following in the debug text over the Client A's head:
- The message "No new data received since ...s" appears, indicating no serialization has occurred.
- In Client A's perspective, notice the following in the debug text over the their own head:
- timesJoined is equal to 3
- (despite the Client A having rejoined the world 4 times, the data can no longer be updated. VRChat servers still appear to retain the value of 2 being stored for timesJoined)
Observed:
- Persistent data becomes frozen, unable to be updated any further, and is no longer syncable.
- UdonSynced fields is contaminated with illegal values (null is never supposed to be in a field of type bool), possibly affecting other Udon program that depend on the integrity of that data.
Expected:
- Persistent data should remain updatable after new fields are introduced to Player Objects.
- Fields of a certain type should not be contaminated/overriden with illegal values for that type.
- A clear guideline about adding new fields to Player Objects should be documented officially.
- According to a VRChat representative on Discord: "in theory it should be just adding udonsynced fields and it's done. There's even metadata that is used to match up an old serialization to a new script to ensure compatibility (and this code has been in use for over a year when two people in the same instance are on different versions of the world). There seems to be some bugs with that system occasionally and it's definitely got room for improvement. Providing a default value rather than straight null would definitely be a good start, and I assume that's 90% of the issues, but it's hard to quantify so I'm not sure exactly what other issues might exist"https://discord.com/channels/189511567539306508/1294086083524432022/1330996987797045320
Log In
EvolvedAnt
Thank you Hai for discovering and posting this, I also encountered it which caused irreversible saved data loss for a number of players.
I just wanted to post a temporary work around that seems to avoid a majority of the issue for anyone else to utilize:
In the OnPlayerRestored() event handler, for any newly added persisted variables that didn't exist in the prior build, before accessing the values you'll want to check whether they are null, and if so, assign a default value.
public override void OnPlayerRestored(PlayerAPI player)
{
if (GetProgramVariable(nameof(isWorldMusicEnabled)) == null)
isWorldMusicEnabled = false;
if (GetProgramVariable(nameof(highScore)) == null)
highScore = 0;
// etc
}
The issue is that when a new variable is added to be persisted that didn't exist before, the variable is by default assigned null, even for value types (like booleans) that should be defaulted to false. The null assignment is probably screwing up the internal serializer, thus breaking saving new data (why it appears to be read only) and then breaking your Udon code when it reads null for value types which should never be null.
Thank you to Merlin for going over this probable technical analysis and work around with me in the VRChat Discord.
One last note, I highly suggest when using PlayerData or PlayerObject persistence, to completely ignore the documentations examples of storing data in individual persisted variables, and instead store all the data you need in a single JSON string that you read/write to.
Sure it's more boilerplate code you have to write initially, but the end result is that you avoid tons of issues/limitations with the current way Persistence works, including this one. As an example of a removed limitation, you can freely add/remove variables in JSON, while if you use the documented recommendation of using individual variables, once you persist a variable/key, it's there permanently forever.
Thank you to Bobystar for explaining this aspect to me.
I believe the issue this canny brings up should still be fixed regardless of the work arounds, because it's an undocumented bug that is easy to encounter unwittingly merely by following the suggested usage of Persistence.
StormRel
tracked