search

lisandroct / EventSystem

Event system for Unity using ScriptableObjects
MIT License
14 stars 1 forks source link

Various fixes related to Event becoming GameEvent #10

Closed paulmasri closed 3 years ago

paulmasri commented 3 years ago

Fixes #9

paulmasri commented 3 years ago

Hold fire! This isn't the only fix needed. I'm about to fix the example too.

paulmasri commented 3 years ago

OK. I think it's ready to go. 👀

paulmasri commented 3 years ago

Nope! Found another bug:

Generated events reference GameEvent<...> instead of Event<...>

paulmasri commented 3 years ago

Sorry — I should have tested that last commit properly before pushing.

There's a problem with AddParentType() because it expects a Type argument that is a non-generic, but then that class name is used to add generics. It wasn't a problem before renaming Event to GameEvent, but now that the non-generic name does not match the generic name, AddParentType() doesn't work properly.

If you give the type GameEvent, it generates classes incorrectly, attempting to derive from lisandroct.EventSystem.GameEvent<...>.

If you give the type Event, it also generates classes incorrectly, attempting to derive from UnityEngine.Event<...>.

I need to give this more thought (tomorrow). @lisandroct Maybe you have a suggestion?

paulmasri commented 3 years ago

Leaving aside AddParentType(), what do you think about changing generic Event to GameEvent too? That way you reunify the naming of all events to consistent naming and avoid conflict with UnityEngine.Event. And there would be no need to change the naming of generated events. e.g. BoolEvent can remain BoolEvent.

lisandroct commented 3 years ago

Leaving aside AddParentType(), what do you think about changing generic Event to GameEvent too? That way you reunify the naming of all events to consistent naming and avoid conflict with UnityEngine.Event. And there would be no need to change the naming of generated events. e.g. BoolEvent can remain BoolEvent.

This is true. I might do that.

lisandroct commented 3 years ago

I closed this by mistake but the fixes were merged. Thank you! I got rid of the 2019 example though.