Systems-Modeling / SysML-v2-Pilot-Implementation

Proof-of-concept pilot implementation of the SysML v2 textual notation and visualization
GNU Lesser General Public License v3.0
123 stars 24 forks source link

ST6RI-482: Render <<perform>> and <<exhibit>> for just `perform` and `exhibit` instead of <<perform action>> and <<exhibit state>> #469

Closed himi closed 1 year ago

himi commented 1 year ago

So far the visualizer did not distinguish perform action and perform or exhibit state and exhibit. So it visualize actions and states with <<perform action>> and <<exhibit state>> for both cases. This PR renders such perform and exhibit with <<perform>> or <<exhibit>>

This PR also fixes the following issues:

himi commented 1 year ago

After this fix

package TestPerformExhibit {
    part p1 {
        action aa {
            perform action pa1;
            then perform a0;
        }

        state ss {
            exhibit state es1;
            then exhibit s0;
        }
    }
    action a0;
    state s0;
}

is rendered in the tree view as Screenshot 2023-02-11 at 4 57 58 PM

And the mixed view renders it as: Screenshot 2023-02-11 at 4 58 41 PM

seidewitz commented 1 year ago

This isn't quite right.

The idea is that, when «perform» is used in the graphical view, then the reference subsetting is not shown, like in the textual notation. That's the point of the notation. Also, when «perform action» is used, then the reference subsetting, if it exists, should be shown using the ::> symbol, not the :> symbol.

For the purposes of the PlantUML rendering, I would suggest that «perform» be used when the declaredName and declaredShortName are empty and there is a referenceSubsetting that provides an effective name (and/or shortName). So, there would be the following three cases for what is shown in the name compartment for a PerformActionUsage (and similarly for an ExhibitStateUsage):

  1. No reference subsetting (perform action a1;): «perform action»
    a1
  2. Reference subsetting with a declared name (perform action a2 ::> a0;): «perform action»
    a2 ::> a0
  3. Reference subsetting without a declared name (perform a0; or perform action ::> a0;): «perform»
    a0

The above notation should be the same on both the tree diagram and the action diagram. Also, on the tree diagram, there is supposed to be a specific kind of specialization arrow for reference subsetting, but that is not critical for now if the reference subsetting is properly identified in the name compartment text.

himi commented 1 year ago

I had that idea too in the second case, but the only difference is just give a declared name. (Edited. I noticed you mentioned declaredShortName as well).

In my preference, I would like to distinguish Cases 2 and 3 from Case 1. Semantically it's the separation rather than just the notation, while I understand it aligns more with the textual notation.

himi commented 1 year ago

BTW, to introduce another edge notation for ::>, we need to change PlantUML itself.

himi commented 1 year ago

I can come up with this pattern as well:

action a0;

part def PP {
    perform action aa;
}

part p2 : PP {
    perform action :>> aa ::> a0;
}

It does not have declearedShortName but uses perform action keyword and has a reference subsetting.

himi commented 1 year ago

I got Sandy's agreement and updated the code to align with the textual notation:

package TestPerformExhibit {
    part p1 {
        action aa {
            perform action pa1;
            then perform a0;
            then perform action <'A-0'> ::> a0;
            then perform action a00 ::> a0;
        }

        state ss {
            exhibit state es1;
            then exhibit s0;
            then exhibit state <'S-0'> ::> s0;
            then exhibit state s00 ::> s0;
        }
    }

    part p2 :> p1 {
        perform action :>> aa.pa1 ::> a0; 
    }

    state s0;
    action a0;
}

gives Screenshot 2023-02-12 at 10 32 09 AM

himi commented 1 year ago

I'm almost screwed up with so many requirements. Finally the example below:

package TestPerformExhibit {
    part p1 {
        action aa {
            perform action pa1;
            then perform a0;
            then perform action <'A-0'> ::> a0;
            then perform action a00 ::> a0;
        }

        state ss {
            exhibit state es1;
            then exhibit s0;
            then exhibit state <'S-0'> ::> s0;
            then exhibit state s00 ::> s0;
        }
    }

    part p2 :> p1 {
        perform action :>> aa.pa1 ::> a0;
    }

    part p3 {
        perform a0 :> a00;
        perform action a1 ::> a0 :> a00;
    }

    action a00;
    state s0;
    action a0;
}

is rendered as:

Screenshot 2023-02-12 at 5 18 52 PM

seidewitz commented 1 year ago

I think the rules are actually fairly simple, as I stated my earlier comment, just based on whether there is a declared name or short name. The intent is to avoid redundant reference subsetting notation like

«perform action»
a0 ::> TestPerformExhibit::a0

In this regard, e.g., perform action :>> aa.pa1 ::> a0; is just the same as perform a0 :>> aa.pa1; – there is no difference in the abstract syntax, other than the order of the subsetting relationships, and neither has a declared name, so I would expect them to be rendered the same way graphically using «perform».

seidewitz commented 1 year ago

@himi Please let me know if you will be able to address my latest comments today, or if we need to defer this PR to the next release. Thanks.

himi commented 1 year ago

Sorry, I'm quite busy today and I do not fully understand your point and let me suggest to defer it. Possibly we need to discuss it in Graphical WG.

seidewitz commented 1 year ago

I made one change: in VStructure::hasRefSubsettingWithoutDeclaredName I changed return f.getOwnedRedefinition().isEmpty(); to return true;. The result is that the example is rendered as below (the only difference is the one action I mentioned in my previous comment):

image

If this is acceptable, then I am ready to approve and merge this pull request.