Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issues when using manual edit with color types - fix 2081345 #179

Merged
merged 13 commits into from
Oct 10, 2024

Conversation

mtygesen
Copy link
Contributor

@mtygesen mtygesen marked this pull request as ready for review September 27, 2024 13:59
@mtygesen mtygesen changed the title Issues when using manual edit with color types Issues when using manual edit with color types - fix 2081345 Sep 27, 2024
@srba srba self-requested a review September 27, 2024 20:08
Copy link
Member

@srba srba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is possible to change to add to a product color type a new color, even if it is used in some place that has tokens. The GUI does not allow this change but the manual edit does. It results in a net which is inconsistent and e.g. the unfolding will fail. To reproduce, make one of the places of the color type assignment, add some tokens to that place. Now go to manual edit, add a new type to the product type and save. The token is now wrong and after pressing M you get an error.

@srba srba self-requested a review September 29, 2024 20:19
Copy link
Member

@srba srba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Open token-ring, open manual edit and add the color "abc" at the end of the Process color type. It will complain, but it should be ok to add it (e.g. it is possible to add it through the GUI without manual edit).

@srba srba self-requested a review October 1, 2024 06:09
Copy link
Member

@srba srba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it still works only partically; e.g. in colored referendum, you can add an additional voter named e.g. abc and save and then open the edit again and delete abc and all works fine.

If you try to do the same in token-ring and add a new name to the color type Process, it will allow to add it but it will not allow to remove it again, even though the color is never used anywhere. This is a problem also with the removal through the GUI.

@srba srba self-requested a review October 1, 2024 19:31
Copy link
Member

@srba srba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Open token-ring, manual edit and try to remove the color Process5 from the colortype. It will fail (as expected) and cancel the dialog. Now open the place dialog and try to remove the five tokens in the place (click on them and press "delete"). This for some reason is not possible. If the manual edit is not performed then the tokens can be easily removed.

@srba
Copy link
Member

srba commented Oct 1, 2024

Another problem, open token ring and manually edit the colortype Couple by adding one more Process component. This is possible but creates an inconsistent net (try to press M) as the place now contains tokens in the initial marking that are of wrong type (contain only pairs and not tripples). In this case the editing of Couple should not be allowed.

@srba srba self-requested a review October 2, 2024 20:47
Copy link
Member

@srba srba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Open token-ring, in manual edit add a new color to Process color type and then try to verify the query. You now get an error due to wrong colortype. Also, if you have in some place color type .all and try to add a new elelement to the colortype, after doing that the number of tokens in the place with .all is not updated.

@srba srba self-requested a review October 3, 2024 14:51
Copy link
Member

@srba srba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to work now fine. The only issue is that the error messages in manual edit are confusing. Open token-ring and try remove Process0 color using the GUI, it will give the correct reasons why this cannot be done. Now try the same using the manual edit and you will get a long list of reasons, some of them are not relevant (like that it is used in a variable for example).

@srba srba self-requested a review October 10, 2024 15:07
Copy link
Member

@srba srba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested and all works fine now

@srba srba merged commit 657a6ad into TAPAAL:main Oct 10, 2024
1 check passed
@mtygesen mtygesen deleted the manual-edit-color-types-2081345 branch November 7, 2024 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants