945 reconstruct and verify architecture - #994
Conversation
It is used only by SettingsMenu.
…-verify-architecture
they use / as a separator.
Renamed create_component(s).
It is not an Interactable. Actually, it should be moved to a new namespace Decorators or the like.
Nodes are searched, not graphs.
Nodes are searched, not graphs.
…rked as obsolete.
Deleted unused methods.
…-verify-architecture # Conflicts: # Assets/SEE/Net/Util/FileSync/FileEvent.cs.meta
…-verify-architecture
…-verify-architecture
…-verify-architecture
…-verify-architecture # Conflicts: # Assets/SEE/Controls/ReversibleActions/AddEdgeAction.cs # Assets/SEE/Game/Avatars/BodyAnimator.cs # Assets/SEE/Game/CityRendering/GameNodeHierarchy.cs # Assets/SEE/Game/SceneQueries.cs # Assets/SEE/UserSettings/UserSettings.cs # Assets/SEEPlayModeTests/TestSEEGame.cs # Assets/SEEPlayModeTests/TestUI.cs # Assets/SEETests/TestGraphProviderIO.cs # Assets/SEETests/TestGraphProviders.cs # Assets/SEETests/TestJacocoImporter.cs
…-verify-architecture # Conflicts: # Assets/SEE/GameObjects/GameObjectExtensions.cs # Assets/SEE/UI/ContextMenu.cs
We now check the precise number of nodes of the JaCoCo GXL file. Fixed spelling of node ID (one bracket too much). Improved documentation.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| (where it must be placed at the first position) and Python modules running at later | ||
| stages of the Axivion CI. | ||
| """ | ||
| from typing import List |
There was a problem hiding this comment.
🔵 Needs a closer look
The change set is extremely large and mechanical, and there are confirmed namespace/documentation inconsistencies that should be corrected before merging.
Pull request overview
This PR reorganizes the SEE codebase’s namespaces and file layout to better reflect and verify the intended architecture (incl. Axivion reflexion analysis support), primarily via mechanical moves/renames/splits with the stated goal of no semantic behavior change.
Changes:
- Re-namespaces and redistributes previously broad areas (e.g., removal of
SEE.GO, splitting controls/actions by kind, movingGame/SceneManipulationtoSEE.SceneManipulation). - Splits
DataModel/DG/IOinto format-/purpose-specific namespaces (e.g.,GXL,CSV,ReportImports) and updates call sites/tests. - Adds/updates Axivion architecture config/scripts and example metric input data (e.g., jacoco CSV).
File summaries
| File | Description |
|---|---|
| Axivion/reduce.py | Extends Axivion reduction node types to include interfaces. |
| Axivion/architecture/rules/setup_final.py | Adds final-step view creation script for architecture/mapping views. |
| Axivion/architecture/dashboard/stop_dashboard.bat | Adds a helper script to stop the Axivion dashboard. |
| Axivion/architecture/config/axivion_config.json | Adds Axivion config layering definition. |
| Axivion/architecture/config/compiler_config.json | Adds compiler/toolchain config stub for Axivion. |
| Data/jacoco/jacoco.csv | Introduces new sample metrics CSV for JaCoCo-related IDs. |
| Data/JLGExample/CodeFacts.csv | Removes old sample metrics CSV content. |
| Assets/SEE/Controls/Modifiers/TooltipContentBuilder.cs | Introduces/updates tooltip text builder used by hover modifier; currently has a namespace placement inconsistency. |
| Assets/SEETests/TestGameObjectDimensions.cs | Updates tests to new extensions namespace; contains an invalid XML doc cref reference. |
| Assets/SEE/Extensions/Extensions.cs | Adds namespace “home” stub for extensions; contains a minor doc-comment typo. |
Review details
- Files reviewed: 197/884 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| namespace SEE.Controls.ReversibleActions | ||
| { |
| /// <summary> | ||
| /// Provides extension methods for game objects (not limited to | ||
| /// game nodes and game edges; there are extensions for Color, too. | ||
| /// </summary> |
| /// <summary> | ||
| /// Tests for <see cref="SEE.GO.GameObjectExtensions"/> regarding dimensions. | ||
| /// Tests for <see cref="SEE.Extensions"/> regarding dimensions. | ||
| /// </summary> |
|
If the available AI credits allow it, I would probably have chosen the Copilot review with the Balanced effort level for this PR. Although most of the changes are mechanical, the restructuring affects a very large part of the project, with many file moves and namespace changes. GitHub itself recommends Lite mainly for routine changes where fast feedback is the priority, and Balanced for larger or more complex changes where a deeper review can be useful. Given the size of this restructuring, a Balanced review might therefore be worthwhile and could potentially catch additional inconsistencies. |
I can do (and just did) that, although I asked already Claude for a deep review locally and ran all editor-mode tests without failure. |
There was a problem hiding this comment.
🟡 Changes recommended
Target-node resolution is incorrect, one class is assigned to the wrong namespace, and two files contain embedded BOM characters.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
Assets/SEE/Controls/Modifiers/TooltipContentBuilder.cs:5
- This class was moved under
Controls/Modifiers, but its namespace places it among reversible actions. Every other class in this directory usesSEE.Controls.Modifiers(for example,ShowHoverInfo.cs:8), andShowHoverInfois its only consumer, so this undermines the namespace-based architecture being introduced here.
- Files reviewed: 197/884 changed files
- Comments generated: 3
- Review effort level: Balanced
| { | ||
| if (gameEdge.CompareTag(Tags.Edge) && gameEdge.TryGetComponent(out EdgeRef edgeRef)) | ||
| { | ||
| return GraphElementIDMap.Find(edgeRef.SourceNodeID, mustFindElement: true); |
| using SEE.GO; | ||
| using UnityEngine; | ||
| using SEE.Extensions; | ||
| using UnityEngine; |
| @@ -1,6 +1,6 @@ | |||
| using Michsky.UI.ModernUIPack; | |||
| using SEE.Controls.ReversibleActions.Drawable; | |||
| using Cysharp.Threading.Tasks; | |||
Reconstruct and verify the SEE architecture with Axivion (#945)
Summary
Reorganized classes and namespaces into smaller, more cohesive ones.
The changes look massive, but they just renames, movements, and splits of existing classes and namespaces. There is (or should be) no semantic changes.
There is still room for more improvements, but because I get merge conflicts whenever I merge the master into this branch, I want to merge the current changes into the master already now.
What changed as a result
Namespaces went from 122 to 137, and 388 files moved to match. The
substantive ones:
SEE.GOis gone (41 files). It was too unspecific to carry meaning inan architecture derived from namespaces. Its contents are now in
SEE.Components.*(GameNodes,GameEdges,GraphElements,Objects),SEE.Factories,SEE.MetricScalesandSEE.Cities.Controls/Actionsis split by kind:ReversibleActions,Modifiers,CodeCityActions,Players,RoomObjects,SpeechInput,MetricCharts.Game/SceneManipulation→SEE.SceneManipulation, which no longerpretends to be part of
Game.DataModel/DG/IOis split by format:GXL,CSV,ReportImports.GameObjectExtensions.csis split into ten focused files underAssets/SEE/Extensions/(GameNodeExtensions,GameObjectScaleExtensions,GraphElementObjectExtensions,GameObjectHierarchyExtensions,GameEdgeExtensions,CodeCityExtensions, and others).Removals
JaCoCoImporterandJaCoCoGraphProviderare superseded by the genericreport-import framework;
JaCoCoParsingConfigmoved toDataModel/DG/IO/ReportImports/andTestJaCoCoReportcovers it.Data/JLGExample/jacoco.xmlwent with them.GO,BoundingBox,GameNodeHierarchy,VRStatus,XRInput,GlobalGameObjectNames,HideSourceCodeAndPaper,TextGUIAndPaperResizerand its editor, plus unused methods on
SceneQueries(GetRootsmoved toits only caller
ChartContent,GetLocalPlayertoWindowSpaceManager).Review notes
The restructuring is mechanical but large. Reviewing it file by file is not
worthwhile; the two places worth attention are the ten new files under
Assets/SEE/Extensions/(a split of a god-file).The Python files for architecture verification are still work in progress. They do not need to be reviewed.