Skip to content
This repository has been archived by the owner on Aug 10, 2021. It is now read-only.

Split up & simplify library #30

Open
Edvinas01 opened this issue Jan 5, 2021 · 5 comments
Open

Split up & simplify library #30

Edvinas01 opened this issue Jan 5, 2021 · 5 comments

Comments

@Edvinas01
Copy link
Member

I've used this library in multiple projects now, it seems that the most useful part is events. I honestly used MutableObject just a few times and its usefulness even then was questionable. If it turns out that the MutableObject functionality is needed, it can be moved to a separate library as not to bloat this one.

Given this, I think the following steps would make most sense (throughout multiple releases):

  1. Deprecate MutableObject functionality.
  2. Finish up improvements for GameEvent and GameArgumentEvent.
  3. Drop MutableObject functionality.
  4. Rename the repository to something something *Events, as the focus is only on events.
@Edvinas01
Copy link
Member Author

I think that GameEvent and GameArgumentEvent should be the same thing. This could be achieved where every event would require an arguments object, where simple GameEvent instances would receive Empty arguments object or something along the lines.

In a situation where each event would receive an object, we could store quite a lot of info in each event argument instance. This would be beneficial for: #27, #23 and #22.

One downside of this approach is that performance might tank, but in contrast it would make the API a lot more flexible.

@KacperKenjiLesniak
Copy link
Contributor

@Edvinas01 Just thought I might share my thoughts as I'm currently using this library a lot 😄

I have also used this library in a couple of projects now and every time I made strong use of the MutableObject functionality, so from my perspective, it would be great to keep it. I know it is more of a wrapper to an existing functionality than a real standalone feature, but it is very handy nonetheless.

As for the second comment, I agree it would make sense to have GameEvent be a special case of an empty GameArgumentEvent and if the Empty argument is not instantiated every time, but just a static shared object like Enum I don't see why it would impact performance.

Another thing is adding additional info to each event argument instance, which might be heavy, so it would be great to have a possibility of switching between a debug mode, where this info is passed along each call as well as a production mode where the arguments are stripped to a minimum.

@Edvinas01
Copy link
Member Author

@KacperKenjiLesniak Its awesome to hear that you're using it! I'm planning on plugging it into my thesis project as well :D

I have also used this library in a couple of projects now and every time I made strong use of the MutableObject functionality, so from my perspective, it would be great to keep it. I know it is more of a wrapper to an existing functionality than a real standalone feature, but it is very handy nonetheless.

I think your argument is sound. I was thinking of how this could be tackled without screwing everyone who relies on this package (even if the number of users is tiny). I'm thinking the best approach right now is to make 2 new packages/repositories - one for the events (almost ready) and one for the mutable objects, then they could be linked in the README.md of this package as a "redirect" for new users.

I feel that combining them doesn't make much sense as they do different things. Also its hard to give a fitting name when keeping them both in the same place :x

Also I'm curious, did you find the ResetType functionality useful?

As for the second comment, I agree it would make sense to have GameEvent be a special case of an empty GameArgumentEvent and if the Empty argument is not instantiated every time, but just a static shared object like Enum I don't see why it would impact performance.

Currently I've setup something along those lines (BaseScriptableEvent is essentially GameArgumentEvent, I decided that this is a more fitting name as someone working on a non-game project might make use of this package):

namespace ScriptableEvents.Simple
{
    [CreateAssetMenu(
        fileName = "SimpleScriptableEvent",
        menuName = "Scriptable Events/Simple Scriptable Event",
        order = -10
    )]
    public class SimpleScriptableEvent : BaseScriptableEvent<SimpleArg>
    {
        /// <summary>
        /// Raise this event without an argument.
        /// </summary>
        public void Raise()
        {
            Raise(SimpleArg.Instance);
        }
    }
}

And the SimpleArg class looks like this:

namespace ScriptableEvents.Simple
{
    public class SimpleArg
    {
        public static readonly SimpleArg Instance = new SimpleArg();

        private SimpleArg()
        {
        }
    }
}

Another thing is adding additional info to each event argument instance, which might be heavy, so it would be great to have a possibility of switching between a debug mode, where this info is passed along each call as well as a production mode where the arguments are stripped to a minimum.

I was hoping that the debug feature could be used to enable/disable this, but currently I'm having a hard time figuring out how the API should look if I wanted to pass around an "event data object". The problem with using an "event data object" is that it makes the API a lot less flexible, it essentially removes the ability to slot in events into Unity UI components such as sliders, etc - the UnityEvent API is not that flexible. Also it generates a lot of garbage. I think this will need more thought and/or some stack-trace hacking, but I don't think that this is a must have feature for now.

@Edvinas01
Copy link
Member Author

Edvinas01 commented Jan 27, 2021

The new "game event" package can be found here:
https://github.com/chark/scriptable-events

With a slim change-log for the upcoming release:
https://github.com/chark/scriptable-events/blob/master/Packages/com.chark.scriptable-events/CHANGELOG.md

Right now its missing automation and its "first" release.

@Edvinas01
Copy link
Member Author

Edvinas01 commented Feb 9, 2021

The first release (second with a hotfix) is up and running at https://github.com/chark/scriptable-events. I've also registered the package on OpenUPM as well for ease of use.

I've closed existing issues on this repository and migrated relevant ones to the scripable-events one. I'll keep this issue open until the MutableObject functionality is migrated. Will comment here after that is done and update README.md.

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

No branches or pull requests

2 participants