I started using the Savable implementation about a week ago and came across something that might be an issue. I have a subclass of Node and in my subclass I have an ArrayList of Spatials called “lights”. When I went to load this object after having saved it, I kept getting errors to the effect that my array had a negative size, then I basically just saved an empty array and got an error that it couldn’t be cast to ArrayList. Well I took a wild guess that it was the name of the object causing the problem and sure enough, I was right. So there must be something saved in a superclass called “lights” that my code ended up loading instead of the object I had saved called “lights”. This seems like it could be dangerous because I don’t always know what objects are being saved further up the tree in the JME superclasses. Had this object been an int for example, named the same, I could’ve easily loaded erroneous data which could’ve caused a very hard to track down bug.
As a user, the only way I can think of to prevent this would be to prefix all of my objects name’s with something unique so there is no conflict. But it seems that perhaps the load/save code in jme should take this issue into consideration and differentiate objects saved in subclasses and superclasses as being different even if named the same. Just a suggestion to avoid some grief.
Nope. It didn’t. I’m guessing that’s because in the superclass the actual object is probably called something like lightslist but in the read/write methods for savable it’s just ‘lights’.
Probably the only reasonable “solution” would be to catch the issue early and prevent you from doing such a thing. Otherwise, we have to bloat every object in a non-backwards compatible way just to let people do something we strongly discourage in the first place (and is apparently pretty rare). That seems unlikely.
At least if you got an error when you were saving you’d know you had a conflicting name.
It’s not about the field names in the class but about the field names that are saved. The IDE can’t know that the write() method used “lights” when storing the spatial’s light list.
That’s one of the many example why we recommend to not override Node or Geometry or Spatial.
A spatial has a list of lights called localLights, and it’s serialized as “lights”.
What’s your use case? why do you store this list of spatials? Maybe we can guide you to a safer approach.
I understand nehon, and you are probably right about overriding Node. I’m manipulating certain spatials in the object, culling them at times and so forth. Anyway, I think actually the easiest solution to this issue would just to make sure that all jme objects are serialized with the same name as the actual object. I think that would help a lot.
No, that breaks every existing j3o just to save one user from a bad thing.
The fields that are saved may be different than the fields in the object for any number of reasons.
The best we can do is to throw an exception if you try to write the same field. That’s the answer. That would have let you know immediately that you were doing bad things the first time you tried to save it.
I only looked for five minutes… but it seems like we could modify this line:
cObj.nameFields.put(name, new BinaryClassField(name, alias, type));
…in BinaryOutputCapsule to throw an exception if put() returns a previous value.
Edit: except I’m wrong. a) there is an earlier check preventing that from working, and b) that cObj is shared across instances. Just need to add a Set<String> to the BinaryOutputCapsule to detect dupes.
Well that’s not that easy. OutputCapsule is an interface, and experters implements it. That would mean we have to change this to handle this for every exporter in a default capsule, or that every exporter has to handle it…
