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

816 layouts for reflexion modeling #824

Merged
merged 116 commits into from
Feb 21, 2025
Merged

Conversation

koschke
Copy link
Collaborator

@koschke koschke commented Feb 12, 2025

This PR introduces a new nested layout for the reflexion analysis where a user can select one layout for the implementation and another layout for the architecture.

To achieve that, the user needs to select "Reflexion" from node-layout menu. The two new sub-menus show up and the user can select the layout for the implementation and architecture, respectively.

It should be noted that the first and original data path relates to the implementation if "From File" was selected as a node layout for the implementation. The architecture layout data path (the second and new one) relates to the architecture.

To enable that this nested layout, a substantial refactoring of the layouts was necessary.:

  • All layout calculations now have parameters for the centerpoint and the width and depth of the available area.
  • There are now no flat node layouts anymore.
  • The distinction between LocalScale and AbsoluteScale for ILayoutNode does not exist anymore (there was never a distinction).
  • The ILayoutNode type hierarchy has been simplified.
  • A NodeTransform is now defined by its center point, no longer by its ground center. This is consistent with Unity and simplifies things.

In addition, the Cose layout was removed. We have never really used it and it was declared as deprecated. Likewise, Manhattan layout was removed. It is not hierarchical and makes little sense. It existed only because it was the very first layout implemented to get things going.

Additional unrelated changes:

  • GraphReader has now (a very simple) progress report.
  • Obsolete code in SEECityRandom was finally removed (existed only for a publication).
  • A bug in EvoStreets was fixed.
  • Several minor code improvements here and there, such as the use of the simplified new() operator.
  • Shortened the time for tests waiting for a response from the Axivion Dashboard.
  • The scene SEENewWorld has changed because the Cose layout was removed.

koschke added 30 commits January 8, 2025 19:18
Added tool tips. Added readonly where possible.
…lexion-modeling

# Conflicts:
#	Assets/SEE/Tools/Livekit/LivekitVideoManager.cs
#	Packages/packages-lock.json
#	ProjectSettings/ProjectVersion.txt
…lexion-modeling

# Conflicts:
#	Assets/SEE/Game/City/SEEReflexionCity.cs
#	Assets/SEE/Game/CityRendering/GraphRenderer.cs
#	Assets/Scenes/SEENewWorld.unity
All other things are still work in progress.
Removed references to removed Cose Layout.
Split ILayoutNode.cs into separate files.
@koschke koschke added the enhancement New feature or request label Feb 12, 2025
@koschke koschke marked this pull request as ready for review February 12, 2025 09:09
@koschke koschke requested a review from Cyclone1337 February 12, 2025 09:09
Copy link
Collaborator

@Cyclone1337 Cyclone1337 left a comment

Choose a reason for hiding this comment

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

Thank you for this really useful feature.
I really like the customization options, such as the size of the architecture and the paddings, etc. However, I think the default should be that there is some space between the two subgraphs (Implementation / Architecture) to allow selecting the main root. For example, to execute a delete action.
Unfortunately, I noticed that this feature is not yet displayed correctly in the RuntimeConfigMenu. There, regardless of the selected node layout setting kind, Architecture and Implementation are always visible and selectable.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

There are a few bad patterns I found which you should check.

Copy link
Collaborator Author

@koschke koschke left a comment

Choose a reason for hiding this comment

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

All fine now.

@koschke koschke enabled auto-merge February 20, 2025 16:35
@koschke koschke disabled auto-merge February 21, 2025 12:56
@koschke koschke merged commit 01bd8f7 into master Feb 21, 2025
3 of 7 checks passed
@koschke koschke deleted the 816-layouts-for-reflexion-modeling branch February 21, 2025 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Different layouts for architecture and implementation in reflexion modeling needed
2 participants