Add bridge configurator with map editor, viewscreens, and auto-assignment#748
Add bridge configurator with map editor, viewscreens, and auto-assignment#748mechatronics-studio wants to merge 16 commits into
Conversation
Brings in Long Range Communications feature. Resolved conflicts in systems/index.ts, cores/index.ts, Select.tsx (kept both branches' additions), and icon files (took develop's generated version; source SVGs for branch icons are present and will regenerate on next build).
Clients can now be automatically assigned to stations based on bridge configuration. Each station element on the bridge map editor has a new "Client Name" field — when a connecting client's name matches, they're locked to that station. Key changes: - Bridge map editor: Added clientName field to station elements, shown below the icon on the map with the station name above - Auto-assign triggers: Runs at flight start, on client connect, and on client name change - Lobby UI: Auto-assigned clients show "Auto-assigned" label, can't be manually reassigned or removed, and are hidden from the unassigned list - Welcome screen: Auto-assigned clients see "Go To Station" instead of "Go To Flight Lobby" - Smart redirect: /flight now routes directly to station/core based on assignment - Server guard: setStation rejects reassignment of bridge-assigned clients - ECS: Added bridgeAssigned boolean to flightClient component - Data model: Added clientName to BridgeMapElement, migrated clientId to clientName in BridgeClientAssignment - Lowered minimum client name length from 2 to 1 character
Resolve conflicts in Plugins/index.ts and bunDataStoreProps.ts to
integrate nested aspects refactor with bridge configurator. Extend
loadAllAspects glob to match manifest.{yml,json} for bridge support.
This reverts commit c55c509.
alexanderson1993
left a comment
alexanderson1993
left a comment
There was a problem hiding this comment.
With a PR this big, there's going to be lots of comments. There are probably some things that I missed in this review that I'll bring up in a future review too. That's just how these things go.
One thing to note that I didn't get in the other comments: the station complement -> flight client mapping seems a bit complex. I think it can be as simple as this:
interface FlightClientStationMap {
clientName: string;
stationComplement: string;
station: string;
}
If this were SQL, this would be a double-composite primary key between clientName and stationComplement. For this, though, I'm okay with allowing duplicate clientName -> stationComplement mappings, since that would be unlikely to happen in practice.
Anyway, Bridges have a list of those. When flight is started, it gets all of the FlightClientStationMap records for the station complement of the ship, creates flightClient entities for each client in the list, and assigns it to the ship and station.
As an aside, I would love to get some kind of view frustum visualization, but as you said in this comment #745 (comment) we can save that for a future PR.
Let me emphasize that this is not how I would have done the bridge configuration, but in many ways the UX of what you've produced is far better. Visually placing the clients and then going through each station set to assign the stations, with a clear visualization of what's been assigned, is great. Well done on that, just lots of adjustments to make it even better.
alexanderson1993
left a comment
alexanderson1993
left a comment
There was a problem hiding this comment.
There's a lot to review here. Offhand, the station assignment wasn't working. If you want something to do while I slowly go through this, feel free to add some automated tests that verify things are working correctly.
In case it wasn't clear, we can assume that any clients assigned to the bridge have already connected to the server, and therefore already have a record on ctx.server.clients. That list of clients can be used both as a dropdown when giving a bridge client a name (no need to manually input the client's name via a text input box), and when assigning flight clients to a station based on a bridge configuration (no need to add expectedClientName to the flightClient ECS component).
|
What do you mean the station assignment wasn't working? I've tested both the manual assignment and auto assignment and they work fine for me. I do need to get into adding automated testing for things. |
|
You know what, I probably didn't assign the bridge when starting the flight. That's on me. I'll try again in the morning. Even still, there's a lot of code and logic here and I think it could be streamlined in the ways I mentioned, and more. |
|
I didn't add a dropdown for existing client names, though they should be created automatically on startup. Happy to add it. I would think we'd want to support a drop-down and free text? A flight isn't guaranteed to be present when someone connects to the server and their client needs to be named something, yeah? "Even still, there's a lot of code and logic here and I think it could be streamlined in the ways I mentioned, and more."- is this exclusively directed toward the auto-assignment handling or a more general comment about the PR? |
|
We're assuming that all clients that will be included in the bridge have already connected once, so their names and IDs are already present on the non-flight server database. No need for drop down AND free text. Just drop down is fine. Re. Code complexity and verbosity, I am referring to the whole PR. |
|
It's not guaranteed that all stations will be assigned in the bridge configurator for all complements unless we enforce it (which it isn't currently). This would mean the drop down list might be incomplete (unless we autogenerate for unassigned stations?). Upon connection, is a flight client orphaned until a client name is chosen from the list. Isn't this starting to encroach on the purpose of the flight lobby? |
|
The purpose of the flight lobby is to assign stations when there is no bridge AND to make adjustments if necessary when there is a bridge. It's a "Yes, and..." kind of thing. It's fine if not every station is assigned. |
2 participants
Description of Changes
Introduces a full bridge configuration system that allows plugin authors to visually design bridge layouts and configure how clients connect to stations during a flight.
Includes server-side CRUD procedures for all bridge sub-resources.
multi-select, resize handles, snapping, and a property editor sidebar for element-specific fields (icon, client name, component, etc.).
Auto-assignment runs at flight start, on client connect, and on client name change. Auto-assigned clients show an "Auto-assigned" label in the lobby and get a direct "Go To Station" button on the welcome screen.
viewscreen core allows the flight director to manage viewscreen state. Clients assigned as viewscreens render the viewscreen card instead of a station layout.
Related Issue
How do you know the changes work correctly?
End to end testing.
Screenshots (if appropriate):