lance icon indicating copy to clipboard operation
lance copied to clipboard

Script minification causes name mismatch in Serializer.registerClass

Open brianchirls opened this issue 4 years ago • 2 comments

The registerClass method uses the name of a constructor function to generate a hash key, which is in turn used to encode and decode data for serialization/deserialization. However, that name can change if the script is minified/mangled/obfuscated or otherwise changed.

There are two ways this can go wrong:

  1. Minifiers will usually change function names to short strings, often a single letter. This makes it more likely that different classes will have constructor functions with the same name, causing hash collisions.

  2. If the client-side script is minified but the server is not, the same class will have different names and therefore different hash keys on the server and the client, causing all incoming serialized objects to be unrecognized.

One solution is to require that all serializable classes/constructors have a unique, static name property/getter. This is not strictly necessary for code that won't be minified, since classes that inherit from other classes should set their own constructor function name, even if the parent class has an explicitly declared name. But it'd be best practice and should be applied to all serializable classes defined in the Lance engine or any other published libraries.

In the meantime, the only workaround I can figure out is to disable renaming functions in minifier settings.

This is related to #82, but distinct. Both need to be fixed separately.

brianchirls avatar Sep 16 '19 19:09 brianchirls

All true. However, the requirement that clients need to provide a unique id seems like a heavy solution. Also, we want this value to be very short, so a randomly-generated uuid (like Java classes do) would be too big.

namel avatar Sep 20 '19 08:09 namel

To be clear, I wasn't suggesting a globally unique id, but rather a deterministic function name to be hashed into an id that's global-ish, within just the code base.

But yeah, a UUID is overkill. Indeed the value of something like that would be if either we had SO many classes that we need that many bits to index them or if we had a number of independent processes indexing classes without consulting each other.

Naturally, it's not reasonable for a real-time game system like this to need billions or even thousands of classes, so we don't need the former. But it seems to me that the current hash method kind of assumes the latter, which isn't really necessary.

i.e. If we required the the client and server register classes deterministically, in the same predictable order, we can just use an an incrementing index and throw out the hash function altogether. 16 bits ought to be plenty, or 32 if you REALLY want to play it safe. You could even apply some simple compression to reduce it to only as many bits as you need.

That would solve both this issue and #82.

brianchirls avatar Sep 20 '19 16:09 brianchirls