Create a StateManager class for an event driven architecture

Philip-Scott commented 5 years ago

Expected Behavior

Akira will be a big app. We need a single way where we can execute different events from different parts of the app, and have different things react to it. Introducing the Event Driven Architecture!

Possible Solution

Let's take for example the Zoom action for the app. There are multiple ways that Zoom can be set. It can change from the Headerbar, via Ctrl +/-, and maybe even via actions like double clicking in an area or Ctrl + Scrolling on the mouse. With the proposed architecture, we hold the state of the whole app in a central place, and "producers" will be the ones changing the property. You will then have certain consumers for the property, the canvas so that it can react to that event, and also the headerbar's zoom button so it can change the sensitive prop and the zoom text. The property changes in the central "bus" and propagates the signal to those consumers who requested to connect to it.

This bus could even be serialized and stored within the file to hold the UI state of the app!

How the architecture looks like

The proposal

One "Bus" per window of the app to handle said state changes. This can hold things like the current selected object(s), the canvas state, the currently opened file, and also would be the place where all the shortcuts would go though. Basically, if it's one part of the UI that needs to comunícate with another, it should go though here.
Not using Dbus, just a regular object. Dbus is for inter-process communication and this is for communicating withing the same window. If we ever need to share info between windows, we can do so via a Application-wide bus

Philip-Scott commented 5 years ago

Alright, so I've been playing around with this sort of architecture over at Notes-Up to find shortcomings and the advantages/disadvantages, and i can say that it works pretty good!

One of the things that I found was that some elements should only be set using actual setter functions instead of having having the usual property { get; set; } properties.
Doing this allows you to have one notify debug connection for easier debugging. Like so:
```
notify.connect ((spec) => {
debug ("Property changed in state: %s\n", spec.name);
});
```
Unlike my example in Notes-Up, it might better to have the State class in maybe the Window object, and just pass that to the rest of the widgets that need it instead of having it as a global parameter. If we do, we should use the unowned. This worked in Notes-Up since by design there is only one instance of the window at a time.
A shortcoming: Each connection to the state class needs to be independent of the rest. Signal order may change from the development of the app, and debugging signal orders can be a bit of a nightmare.
- But having all signals in one place also allows you to debug easier, since you can also just connect to it's events from the State class, and also set the visibility to private and try to compile so you see were it's currently in use
The Action Manager Class we have might be a great place to put this state manager. For example, action_zoom_in should change the state instead of calling the headerbar's function window.headerbar.zoom.zoom_in ();. We can always make smaller state classes like CanvasState or WindowState to group similar behavior in one file

giacomoalbe commented 5 years ago

I'd like to add my two cents to this conversation.

I do think that this approach is well suited for the purpose of Akira.

I do use an Event-Driven bus for every web application I work on, and I do this especially for making different part of the application communicate together without the need to store a reference to a common object (the main window, in Akira terms) shared by every sub components.

In the PR I proposed (#124) there is already a basic implementation of this technique.

I created a Class called (with not too much creativity :) ) EventBus which is shared between all the components in the Akira namespace (just like settings).

A component can emit signals using this bus and other elements/components can listen to these events and trigger different actions based on the nature of the signal.

Since everything is triggered from the emit method of this bus, there you can change the state of the application and after that change emit the signal you where calling the emit function for.

Consumer components, then, can listen to event_bus events and trigger action accordingly.

Let's take the zoom example again to explain this better. In the ZoomComponent, upon user click on the button, the controller emits a signal request like that:

zoom_button.clicked.connect(() => {
    // data is a map of certain type to store values that needs to be sent
    event_bus.emit("zoom_in", data);
    // Update the UI according to this event, eg. change label 
     update_ui();
});

Then in the event_bus emit:

public void emit(string signal, signalData data) {
    switch (signal):
          case 'zoom_in':
                 actual_zoom = data.zoom;
                 CanvasManager.zoom(data.zoom);
                 zoom_in_signal(data);
                 break;
}

In here, the event is processed (all in one place, no fragmentation of action) and then the actual signal is emitted (zoom_in_signal) and everywhere else this signal is needed, the proper actions can be triggered (update status_bar or other UI components).

I think that this model is somehow similar (or on the same wavelength) with the draft you proposed, I hope this might help the project :)

Philip-Scott commented 5 years ago

The only thing that i don't really agree with is having everything pass though the event_bus function instead of just having separate functions such as a zoom_in_action. This will make everything just use one calling point, which will make the emit function super large and also if in the future we need to add/remove parameters, or even try to refractor it this could complicate things.

Instead, by separating action types like UI, Canvas, CanvasItem, and having things as it's own separate functions, we can change the internal state in one place while also sending the needed signals on each function (And the event code wouldn't be 1000s of lines long) :)

Alecaddd commented 5 years ago

This bus could even be serialized and stored within the file to hold the UI state of the app!

I love this idea!

Thank you both for this great start and conversation. The EventBus approach is interesting, but I don't think it will scale properly for everything that Akira will need/have in the near future.

Let's image for example the simple selection workflow of a rectangle shape. When the shape is selected:

The shape properties panel will appear in the left panel
The layer in the right panel will be selected and the layers panel will scroll to center
All the toolbar buttons need to update accordingly (if it's not a group, the ungroup button should be disabled, etc)
All the alignment buttons need to update accordingly (If only 1 object is selected, the distribute buttons should be disabled)
and more...

Would all these actions been triggered from within the same EventBus, or should be separate in a dedicated ShapeManager, or should be triggered from the CanvasItem itself?

Philip-Scott commented 5 years ago

So one thing we should differentiate is between a change of state, and an action being triggered. Actions change state, and a change of state can be listened by the different components to react accordingly. The EventBus looks more like an ActionBus, but we still need something to hold the current state :)

In the case you mention with the selection, we could hold an array of the selected shapes* inside of the CanvasState class, and when it's set, each subscriber can react accordingly. It would be the Canvas' work to make the array of the elements selected, and then we would call a set_selection function inside the CanvasState class. This function can then trigger the needed signal, and whatever needs to respond to a change of selection can just hook up to this signal. So in other words, the CanvasState class has no idea that a shape properties panel exists, and it will be the panel's job to react property to the change of the state.

(plural, since we need to also think when more than one shape is selected)

giacomoalbe commented 5 years ago

I totally agree with @Philip-Scott explanation.

When the user does something on the Canvas, the object responsible for managing the Canvas (which, I guess, is the CanvasManager or something) simply updates the Canvas internal states (so it actually moves the object inside it) and then simply calls the event_bus.emit('object_selected') (or as @Philip-Scott proposed previously the event_bus.object_selected_signal (**args) - is pseudo code, bear with me) and then everything that should be notified of this change (UI, panels, layers, buttons, icons, etc) listen to this signal and act accordingly.

This way everything is de-coupled, we don't have too long switch - case scenarios (thanks @Philip-Scott) and we can add up as many components reacting to a change as we want without the need to update the signal emitter. In Italy we say "due piccioni con una fava" (which can be transposed and translated as "getting 2 things with 1 unit of effort only").

zenithlight commented 5 years ago

The event bus idea sounds a lot like Redux reducers. They handle the "too big reducer" problem by splitting it up into separate functions, and using a higher-order function to combine them into one.

One nice advantage of a reducer system is that it becomes very easy to handle things like undo/redo.

Alecaddd commented 4 years ago

How's the situation of this? @giacomoalbe and @albfan, it seems that our current architecture is pretty scalable and easy to manage. I think we can close this, right?

akiraux / Akira