Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Script minification causes name mismatch in Serializer.registerClass #125

Open
brianchirls opened this issue Sep 16, 2019 · 2 comments
Open

Comments

@brianchirls
Copy link

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.

@namel
Copy link
Member

namel commented Sep 20, 2019

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.

@brianchirls
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants