Gohla / serde_flexitos

Flexible (de)serialization of trait objects with Serde
Apache License 2.0
5 stars 1 forks source link

Able to replace or ignore new deserializers #2

Closed Shute052 closed 1 month ago

Shute052 commented 2 months ago

I'm curious about the reason behind the restriction that a Deserializer can only be registered once.

I've implemented a static mut Registry for an input manager for Bevy as a global registry for inputs and used Plugin to add registrations, following Bevy's conventions. However, I've encountered the issue where multiple registrations aren't supported.

Gohla commented 1 month ago

Sorry for the late reply, wasn't able to check GitHub in the past week. The serialized representation of a trait object will contain the identifier for the concrete type of that trait object. For example:

[{"Foo":"A"},{"Bar":0}]

indicates that we have a list of trait objects, with the concrete type of the first trait object being Foo with contents "A", and concrete type of the second trait object being Bar with contents 0. These identifier are there so that we can find the corresponding deserializers for Foo and Bar when deserializing these trait objects.

However, if we would register two deserializers for identifier "Foo", then at deserialization time, we wouldn't know which of these deserializers we would need to use. Therefore, we disallow this. Since handling errors in static initialization is awkward, the Registry::register method does not return an error, but instead causes deserialization to fail at runtime. Is that the restriction you were asking about?

Also, cool that you're using this in your project! Let me know if you run into other issues or have other kinds of feedback, I still want to improve this library.

Gohla commented 1 month ago

Ah, I just read the title of the issue 😅 I'm open to adding a Registry implementation that replaces/overrides existing registrations instead of disallowing that.

However, this can lead to issues where the registration order determines which deserializer is used, especially if you do global registration with libraries such as linkme, which can lead to random/hard to debug issues. But I don't know exactly how registration in Bevy works, so you might not run into that problem at all. Can you give an example/use case where you'd like to replace a registration?

Also note that you can implement your own Registry if you want other kinds of custom registration behaviour.

Shute052 commented 1 month ago

Thanks for getting back to me.

The reason I opened this issue is that the project I mentioned is a plugin, which means it could be added multiple times in one app. This led us to overlook the potential for multiple registrations to cause problems.

However, I believe we can close the issue now, because we've created a custom Registry in that project that disregards the MultipleRegistrations error, and there's no need to introduce such a mechanism into this repo

Shute052 commented 1 month ago

Can you give an example/use case where you'd like to replace a registration?

That project is a plugin that provides input abstractions (e.g., keyboard, game controller and mouse input) and actions (e.g., player movement, firing, etc.) to a Bevy app. We used serde_flexitos to ser/de the implementors of two traits, UserInput (the trait for inputs mentioned above) and CustomInputProcessor (e.g., special deadzones).

Initially, we used enum variants to represent different input types and processors. But recently, we've transitioned to a more flexible trait-based design to enable users to create their own custom input types. This involves using a static mut Registry to store registered trait deserializers when adding the plugin to the app.

And the problem is the plugin is registered for each action enum created by end users. This means that if you have multiple action types (different situations require different actions), you'll need to add multiple plugins to your game app, leading to multiple registrations.

Gohla commented 1 month ago

Thanks for the example, I understand it now. These duplicate registrations are not a problem in your case because the duplicates use the exact same deserialize functions, so they can be safely ignored.

A custom Registry is indeed a good way to handle that. I will update the documentation with this info and add an example with a custom registry.