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

Bug in World Subscription / Serialization functionality? #127

Closed
flomar opened this issue Aug 25, 2021 · 5 comments
Closed

Bug in World Subscription / Serialization functionality? #127

flomar opened this issue Aug 25, 2021 · 5 comments
Labels
bug Something isn't working

Comments

@flomar
Copy link

flomar commented Aug 25, 2021

I'm not sure if it's a bug or if I'm doing something horribly wrong-- please correct me if necessary.

I'm using the integrated TextSerializer to serialize and deserialize the complete state of my application, and it has been working without problems. At some point though I decided to use World.SubscribeComponentChanged<T>(foobar) to implement a callback mechanic. I made some quick tests, and the callback mechanic was working just fine. Unfortunately, it broke the serialization mechanic.

The serializer throws an exception and complains about duplicate key entries for each component I register with SubscribeComponentChanged. If I don't subscribe, everything's working fine again.

Maybe I'm doing something wrong, I don't know. Disposing the subscriptions before serialization doesn't help either, and I couldn't find any relevant information in the documentation.

Can anybody reproduce this issue?

EDIT

To clarify what I'm trying to do here... I want to be notified whenever an entity is changed. I haven't found a way to implement this other than using World.SubscribeComponentChanged<T>(foobar). I guess I could also explicitly add an UpdateComponent or whatever to the entity whenever I change one of its components and create a dedicated system to react properly, but I'd prefer the implicit solution as it would be cleaner and much less error-prone in my opinion.

@flomar
Copy link
Author

flomar commented Aug 25, 2021

On a related note, if I wrote a custom serializer based on Newtonsoft.Json (which I have been using successfully with my own crappy ECS implementation before switching to DefaultEcs), wouldn't that potentially cancel out the limitation regarding circular references of the DefaultEcs serializer?

@Doraku
Copy link
Owner

Doraku commented Aug 25, 2021

That's weird, it seems a lot like a previous issue (#88) which has been fixed a long time ago. I will try to see if I can reproduce it :/
The way you are doing it is definitely what is intended and you shouldn't have to rely on some UpdateComponent, this is mostly a bug.

On a related note, if I wrote a custom serializer based on Newtonsoft.Json (which I have been using successfully with my own crappy ECS implementation before switching to DefaultEcs), wouldn't that potentially cancel out the limitation regarding circular references of the DefaultEcs serializer?

My serializers have some drawback (like the circular reference) but also some perks like being able to serialize absolutely anything, private, readonly even without any attribute decorations on types (which you can't always add on your dependencies). I tried to make it extensible specifically so you can use your own prefered serializer like json if you need to :)

@flomar
Copy link
Author

flomar commented Aug 26, 2021

Thanks for your reply.

Since my overall application is fairly complex, I created a minimal reproducible example:

public class Example {
    public void Run() {
        DefaultEcs.World world = new DefaultEcs.World();
        var entity = world.CreateEntity();
        entity.Set<int>(9999);
        world.SubscribeComponentChanged<int>((in DefaultEcs.Entity _entity, in int _old, in int _new) => {
            Console.WriteLine("Component changed!");
        });
        entity.Set<int>(8888);

        // The serialization below throws an exception-- unless the subscription above is removed.

        var serializer = new DefaultEcs.Serialization.TextSerializer();
        using (var stream = System.IO.File.Create("DefaultEcsExample")) {
            serializer.Serialize(stream, world);
        }
    }
}

Fortunately (or unfortunately, depending on your point of view) it does not seem to be a problem with my application code. Obviously something's wrong behind the scenes. Glancing over the old issue you linked (#88), I couldn't find anything suspicious-- but you're probably way faster at finding the root of the problem than me anyway.

@Doraku Doraku closed this as completed in 169ea30 Aug 26, 2021
@Doraku
Copy link
Owner

Doraku commented Aug 26, 2021

Thx for the small repro, I don't understand how it slipped past me and didn't fail earlier.

@flomar
Copy link
Author

flomar commented Aug 26, 2021

You already fixed it? Great! :)

And you already updated the NuGet package to 0.16.2? Damn... you're fast.

Glad I could help.

I already removed all that UpdateComponent crap I added to my application code yesterday. I invoke World.SubscribeComponentChanged now using generics with each and every component type I do have in my code base, and everything works as intended-- most of all the serialization no longer throws an exception.

Thank you very much!

@Doraku Doraku added the bug Something isn't working label Aug 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants