ObjectProfile / Roassal2

Agile Visualization Engine for Pharo and VisualWorks
http://AgileVisualization.com
MIT License
26 stars 20 forks source link

Loading Roassal2 to Pharo 9 causes errors in Calypso #63

Closed JanBliznicenko closed 3 years ago

JanBliznicenko commented 3 years ago

Hello, Roassal2 has dependency on Geometry project from https://github.com/peteruhnak/geometry. Modified version of Geometry project has been recently adopted by Pharo ( https://github.com/pharo-contributions/Geometry ) and integrated into the main image. Roassal3 depends on this new Geometry repository from pharo-contributions and the same Roassal3 is used for drawing diagrams in Calypso.

Loading Roassal2 causes replacing the new Geometry repository by the old one, causing Roassal3, and therefore Calypso, to not work properly and causing errors when using the Calypso.

Steps to reproduce:

  1. Download and open fresh Pharo 9 image
  2. Load current Roassal2 into the image
  3. Open Calypso (System Browser)
  4. Find and select (click on) any Baseline package (for example BaselineOfAthens)
  5. Select (click on) the Baseline class (for example BaselineOfAthens)
  6. Click on tab Baselines-Graph Result: Error "Instance of GEllipse class did not understand #center:vertex:coVertex:" Note that sometimes the error occurs even before the step 5 or 6.

It is possible that Roassal2 would work even with current implementation of Geometry, but that would require multiple modifications. @hernanmd attempted to solve this by making fork of Peter Uhnak's repo and renaming everything to prevent names conflicts in his fork of Roassal2: https://github.com/hernanmd/Roassal2/commits/master His solution seems to be working.

hernanmd commented 3 years ago

Yes, I can confirm the issue. Actually the only way to get rid of the errors in Calypso was to disable the UML in Settings. I did forked and renamed the classes plus some methods in the version of Geometry which is used by Roassal2. Part of it could be automated and some parts not, so here it is the relevant repository: https://github.com/hernanmd/geometry

bergel commented 3 years ago

I have not tried, but maybe there is no need to load Geometry in the baseline of Roassal2.

JanBliznicenko commented 3 years ago

It is used by RTBorderAttachPoint - directly and indirectly via methods used only by RTBorderAttachPoint.

akevalion commented 3 years ago

I have remove the dependency to old geometry, currently roassal2 depends on https://github.com/pharo-contributions/Geometry

Do you agree with the changes?

bergel commented 3 years ago

Can you maintain this dependency when installing on Pharo 8? It should be easy to have this behavior.

JanBliznicenko commented 3 years ago

It seems to work fine in both Pharo 8 and 9, great. Maybe locking Pharo 8 version of Roassal2 (I mean using specific tag/branch when loading to Pharo 8) would be neccessary soon, just like with Pharo 7, but I don't see that as major issue.

akevalion commented 3 years ago

Tests are green now I will create a new release

bergel commented 3 years ago

Well done Milton!

JanBliznicenko commented 3 years ago

I noticed one issue. RTBorderAttachPoints cannot be created between elements where one of them has 0 size. It is usually not a problem, but I had to increase size (from 0) of my invisible "helping" boxes (where line has to go from or to empty space)