Entering nested states bug. Understanding Partitions.

[gugomea]

Hi, I would like to show a bug when entering nested states in depth ≥ 1 (supposing the initial state is at depth 0) and some questions about partitions and the datadict. The code is from the master branch of the gitlab repository, I'm sorry but currently I cannot access the gitlab account.

• About entering nested states with depth ≥ 1: When we double click on one nested state at level 0 (inside the process) we don't face any issues. But, if inside this nested state, we have another nested state, it will prompt us with the message even though we have previously populated that state with a valid state machine.

As a quick temporal patch, I've changed the following function at opengeode/sdlSymbols.py:1153

def double_click(self):
    ''' Catch a double click - Set nested scene '''
    # We have to look in all partitions, not only in the current scene
    try:
        top_scene = self.scene().views()[0].top_scene()
    except IndexError:
        # there may be no views, for instance if the tool is
        # called with the CLI option to render PDF/PNG/SVG
        top_scene = self.scene()
    partitions = top_scene.partitions
    for part in partitions.values():
        for each, value in part.composite_states.items():
            if str(self).split()[0].lower() == str(each):
                self.nested_scene = value
                return
    self.nested_scene = None

to

def double_click(self):
    ''' Catch a double click - Set nested scene '''
    stateName = str(self).split()[0].lower()
    self.nested_scene = self.scene().composite_states.get(stateName)

We probably don't have to check all partitions, since the partition we would find will point to self.scene(). The reason of the bug is that we only have access to top_level partitions, so we are always checking the depth 0 states no matter how deep we are in reality. This results in iterating until reaching the self.nested_scene = None statement and triggering the prompt of creating the nested state.

• About partitions: I think I don't understand partitions (since I'm new to the GUI world and Qt), so I wouldn't want to submit a merge request trying to fix something that is right because of some misunderstanding.
  1. What is the purpose of partitions, allow programmers to separate pieces of code for clarity purposes?
  2. Since SDL_Scene.partitions is a map of the kind partition_name → SDL_Scene, why is only top_level scene populated with partitions? We don't have enough information, since there could be deeper scenes with same partition name and would wipe past entries in the map.
  3. This question is linked to the previous, when we display the datadict, Do we want to show all states and partitions (the ones in nested scenes) or just the ones at the current level? This might be linked to the TODO of the context ast that currently is a constant, but I'm not sure.

[maxime-esa]

I see the regression indeed, I will check it deeper and come back to it in a few days (and will provide answers to your other questions). Thanks for reporting.

[maxime-esa]

The partitions are a graphical mean to cut the main (top level) diagram in pages. Most of the times SDL diagrams are flat (no nested states) and therefore the diagram can become very large and inconvenient to read.

With partitions you can put for instance one state per page (and name the partition with the name of the state). Navigation between them is then much better to handle.

Because SDL allows to define the same state at multiple places in the same diagram (or partition) - also for readability purposes, it can be that the nested state is defined in one partition, but another partition references the same state. This is why, when you double click on a state, it has to look in all partitions for a possible nested definition. It may not always be defined in self.scene().

The bug you found however is that we should still first look in the current scene, and then if not found, look in the other partitions ; otherwise as you point out, partitions can be at an upper level and miss the sub-sub-state definition. The regression was introduced with the partitions, a few months ago. Unfortunately not caught by the CI as the test being on the GUI side, it's very difficult to automate.

I fixed it on gitlab, and I will merge it in github soon (we have a certificate renewal issue at the moment on gitlab so it will take a few more days).

To answer your other questions:

1. it's only the top-level scene that has partitions indeed and this is a choice made from the fact that most of the time, it is the only one that is large enough to require a cut in pages. Substates are often smaller (and in general less known/used) and fit in a single diagram. This can evolve in the future if needed.
2. Currently only top-level states appear in the data dictionary, but this can arguably be seen as a bug. When entering a substate, the entry in the dictionary should probably be updated. Will add to the TODO list :-)

[maxime-esa]

The fix is committed in the Github repo.

[gugomea]

Thank you for the detailed response, now it's much more clear!