Odonno / ReduxSimple

Simple Stupid Redux Store using Reactive Extensions
http://www.nuget.org/packages/ReduxSimple/
MIT License
144 stars 20 forks source link

Help with understanding subreducers and nested state #94

Closed lewiji closed 3 years ago

lewiji commented 3 years ago

Hi, I'm using ReduxSimple for a game development project, so far it's great.

I'm at a point of complexity where I want to split my root store into several smaller stores + reducers.

In my case this is a turn based sports game and so at the moment I just want to split my state into:

So I have split my store, actions and reducers into 2 files, TurnStore.cs and TeamStore.cs. I combine these in my RootState.cs which just holds an instance of TurnStore and 2 instances of TeamStore as its members and initial state.

What I'm having trouble with is dynamically creating 2 instances of the TeamStore and attaching the TeamReducer in a sensible way without duplicating code.

I have tried a few approaches I found in the sample code and in issues/pull requests.

By following just the README instructions around subreducers as best I could, I got an error saying ~"could not find feature reducer for TeamState in RootState".

I tried an example from the tests around Nested array state but got an obselete warning/error.

Finally I used the "explicit" method found in one of the examples and similarly in a pull request from late last year around this issue. By putting two arguments into the selector for the subreducer, firstly the selector and secondly an in-line function that sets a Teams array on the root state at the right index.

The last attempt worked best (didn't crash) however my actions would not update the state; on debugging, when the reducer was applied, the action's payload wasn't merged into the original state and so the deep equals comes back true and no updates are triggered.

It's late here but I will try to include some sample code tomorrow when I can, mainly I'm hoping someone can point out some advice or a good implementation for something like this?

Thank you 😊

Odonno commented 3 years ago

Hi Lewis,

Indeed, if you try to reuse reducers in this manner. The implicit cannot detect automagically which nested state you try to update and then it throws an exception. However, the explicit should handle this without any problem.

On the reducer not updating state, I don't know exactly why. A reproduction example would help me to put some more unit tests and find out if there is and where is a problem.

Thank you for your feedback.

lewiji commented 3 years ago

Thanks, @Odonno - I'm going to have another go at it this evening, and if I'm still having trouble, I'll create a minimal repro project. I assume I'm missing something obvious 🙂

Odonno commented 3 years ago

Well, you still need to combine reducers and put it in the ReduxStore. Other than, I don't see what you could have missed.

Anyway, don't try too hard, it's possible the problem is on my side. :)

lewiji commented 3 years ago

So, having dug into it a bit more, the issue is definitely with my CreateSubReducers second argument, i.e. the feature reducer inline function which explicitly sets the state.

My RootState looks like this:

namespace WoodOwl.state
{
       public class RootState
    {
        public Turn.State Turn { get; set; }
        public Team.State Teams { get; set; }

        public static RootState InitialState =>
            new RootState
            {
                Turn = state.Turn.State.InitialState,
                Teams = Team.State.InitialState,
            };
    }

        public static class Reducers
    {
        public static IEnumerable<On<RootState>> CreateReducers()
        {

            return CombineReducers(
                Turn.Reducers.GetReducers(),
                Team.Reducers.GetTeamAReducer(),
                Team.Reducers.GetTeamBReducer()
            );
        }
    }
}

And my TeamState:

namespace WoodOwl.state.Team
{
    public class TeamState {
        public ImmutableArray<int> Players { get; set; }
        public int MovedPlayers { get; set; }
        public int PassingTo { get; set; }
        public int SelectedPlayer { get; set; }

        public static TeamState InitialState => new TeamState
        {
            Players = ImmutableArray.Create(0, 1, 2, 3),
            MovedPlayers = 0,
            PassingTo = -1,
            SelectedPlayer = -1,
        };
    }
    public class State
    {
        public TeamState TeamA;
        public TeamState TeamB;

        public static State InitialState =>
            new State
            {
                TeamA = TeamState.InitialState,
                TeamB =  TeamState.InitialState
            };
    }
...

In the interest of trying to get this working, I have 2 functions GetTeamAReducer and GetTeamBReducer which have identical .On lists, just copy and pasted so there's no sharing.

As for the feature reducer function, I've tried a few things, I'm not entirely sure how to get this nesting to update.

public static IEnumerable<On<RootState>> GetTeamAReducer()
        {
            return CreateSubReducers(
                    Selectors.SelectTeamA,
                    (state, teamState) =>
                    {
                        return state.With(new
                        {
                            Teams = state.Teams.With(new { TeamA = teamState })
                        });
                    })
                .On<IncrementMovedPlayersAction>(
                    (state, action) =>
                    {
                        if (Store.State.Turn.ActiveTeam == 1) return state;
                        return state.With(new
                        {
                            MovedPlayers = state.MovedPlayers + 1
                        });
                    })
...
             .ToList();

That's a bit of a mess because I wanted to hook up the debugger to specific parts of that reducer function.

Basically, what I'm seeing is the action that updates the active team is triggered, and the 4 players on that team who subscribe to that state all receive an update.

On selecting a player I then see the explicit reducer function of CreateSubReducers being called, state holds the initial state, and teamState indeed holds the new updated state with the selected player's ID as expected. I can't seem to get the state.Teams.TeamA or state.Teams.TeamB objects to merge with the new teamState. I feel like nesting the .With statements is probably wrong, though I've tried some other things without that. (I'm pretty new to C#, though not programming in general, it's possibly I'm missing something with the quick object initialiser).

The weird thing is that the players receive an update to their subscription, and then I get a NullReferenceException, because for some reason, the state now looks like the below in the debugger:

image

So the feature reducer function is setting both TeamA and TeamB to null somehow.

I think that maybe this is just a limitation or misunderstanding of the .With function, and there may be a simpler way to do this? When I broke the reducer function up so that it did the two .With calls as separate statements rather than nested, the first call (to set TeamA = teamState with var something = state.Teams.With(new {TeamA = teamState}), something did indeed evaluate to null.

Hope that's enough info, if not I can try and make a repro project, but because this is a Godot game engine project it's a little tricky without removing the dependency on Godot. Thanks for any assistance, and boy it'd be nice if I'm just being very stupid and you could point out where :D

lewiji commented 3 years ago

Ohh, OK. If I change the reducer callback to this (naive approach without using any data merging methods):

 return CreateSubReducers(
                    Selectors.SelectTeamA,
                    (state, teamState) =>
                    {
                        state.Teams.TeamA = teamState;
                        return state;
                    })

That actually works. It feels a little weird in terms of mutability but I guess I didn't account for that when using a class (TeamState).

Is there something wrong with this approach? I wonder why it didn't work with With.

Odonno commented 3 years ago

Well, if you break immutability, you will lose some features like the time travel. So, yes, it works. I do not recommend it but if it's ok for you, then keep doing it.

About the fact that With is not working, it's strongly possible that if the object is null, then every attempt to use the With will give a null value. Can you check your object and try to avoid null values? (for Teams or TeamA/TeamB properties)

lewiji commented 3 years ago

Hmm, time travel would be nice, (undoing an action?) it's not essential at this point though. Mainly though it just feels wrong :) But hey, I'm happy it works.

Sure, I am just checking that now, I'm pretty confident there are no null values involved.

Root state:

image

New feature state (there are no nulls in either of the Players ImmutableArrays either btw):

image

Feature slice of root state + .With new feature state (nulls both TeamA and TeamB:

image

So then when it finally tries to merge that .With the root state, it overwrites the teams with null.

If I step into and through the var newState = ... .With ... call, it's a little obscure what's happening in the debugger.

Firstly I immediately see in the debugger watch list that object is correctly set to the Team slice with the old TeamA and TeamB values.

However the propertiesToUpdate says in the watch list:

There is no member 'TeamA = {TeamA'

Which is weird, it could be a red herring of the debugger messing up though?

list1 gets set to the Team.State.InitialState, and list2 gets set to the TeamA property. The properties after that were difficult to view usefully in the debugger. Below is a partially expanded version of the watch list in case it's useful (the variables in .With are renamed V_0 through V_9)

image

If I step into the CreateNewObjectApplyingUpdates call, it doesn't look promising, seems to say there are no props to set, and a repeat of the error on propertiesToUpdate (maybe not a red herring after all).

image

So it seems to be not picking up that TeamA is a property of the Teams slice, and I don't think I'm referencing it incorrectly in my original call?

Odonno commented 3 years ago

Oh god, just see the problem, you forgot the getter and setter here:

public TeamState TeamA { get; set; }
public TeamState TeamB { get; set; }

I also forgot them sometimes and it's really difficult to find the problem at first sight. If you do not provide them, the property will not have public getters and setters and so the .NET Reflection will not be able to do its job (applying property update in the With function).

Can you add them and retry using the With function?

lewiji commented 3 years ago

Omg! You know, I did the same and forgot them on one of the properties of the TeamState class, but didn't even register them on the state! Facepalm.

Yes, it works as expected using .With now. Phew!

Thanks so much for your patience and help.