Split up & simplify library #30

Activity

1. Edvinas01 commented on Jan 18, 2021

   

   Edvinas01

   on Jan 18, 2021

   MemberAuthor

   More actions

   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.
2. KacperKenjiLesniak commented on Jan 26, 2021

   

   KacperKenjiLesniak

   on Jan 26, 2021

   Contributor

   More actions

   @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.
3. Edvinas01 commented on Jan 26, 2021

   

   Edvinas01

   on Jan 26, 2021

   MemberAuthor

   More actions

   @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.
4. Edvinas01 commented on Jan 27, 2021

   

   Edvinas01

   on Jan 27, 2021

   Last edited by Edvinas01

   MemberAuthor

   More actions

   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.
5. Edvinas01 commented on Feb 9, 2021

   

   Edvinas01

   on Feb 9, 2021

   Last edited by Edvinas01

   MemberAuthor

   More actions

   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.