Expand Scenario Management Plugin #1249
Conversation
…tering - Add satellite and sensor list support to scenario files - Collect visible satellites (DetailedSatellites and OemSatellites) when saving - Collect active sensors when saving scenarios - Filter satellites using ObjectGroup system when loading scenarios - Filter/activate sensors when loading scenarios - Support both DetailedSatellite (TLE-based) and OemSatellite (ephemeris-based) satellites - Add toast notifications for save/load operations showing counts This implements STK-style scenario management where scenario files contain lists of satellites and sensors that should be visible when loaded.
|
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Clear current sensors | ||
| sensorManager.currentSensors = []; | ||
| sensorManager.secondarySensors = []; |
There was a problem hiding this comment.
applySensorFilter_ zeroes out currentSensors/secondarySensors (lines 539-541) and then relies on getSensorByObjName in the subsequent loop to resolve each scenario sensor name, but that lookup only searches those same arrays plus stfSensors. Once they are emptied no normal sensor can be found, so loading a scenario with sensors always warns “Sensor not found” and leaves sensors inactive instead of restoring them from the file.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull Request Overview
This PR expands the Scenario Management Plugin to support STK-style scenario management by adding satellite and sensor persistence to scenario files. When saving scenarios, the plugin now captures which satellites and sensors are currently visible/active, and when loading scenarios, it restores that selection state.
Key Changes:
- Extended scenario file format to include satellite and sensor lists
- Added collection methods to capture visible satellites (both DetailedSatellite and OemSatellite types) and active sensors when saving
- Implemented filtering logic to restore satellite visibility and sensor activation when loading scenarios using the ObjectGroup system
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Clear current sensors | ||
| sensorManager.currentSensors = []; | ||
| sensorManager.secondarySensors = []; | ||
|
|
||
| // Set sensors from the scenario | ||
| for (const sensorName of sensorNames) { | ||
| const sensor = sensorManager.getSensorByObjName(sensorName); |
There was a problem hiding this comment.
The sensors are cleared before attempting to look them up, which will cause getSensorByObjName to always return null. The method getSensorByObjName only searches through currently active sensors (currentSensors, secondarySensors, stfSensors), not the full sensor catalog.
Instead, sensors should be looked up from the imported sensors catalog object before clearing, or the lookup should be done from the catalog directly. For example:
// Import sensors at the top
import { sensors } from '@app/app/data/catalogs/sensors';
// In applySensorFilter_:
const sensorsToActivate: DetailedSensor[] = [];
for (const sensorName of sensorNames) {
// Look through the catalog
let found = false;
for (const sensorKey in sensors) {
if (sensors[sensorKey].objName === sensorName) {
sensorsToActivate.push(sensors[sensorKey]);
found = true;
break;
}
}
if (!found) {
errorManagerInstance.warn(`Sensor ${sensorName} not found in catalog`);
}
}
// Clear current sensors
sensorManager.currentSensors = [];
sensorManager.secondarySensors = [];
// Add the sensors
for (const sensor of sensorsToActivate) {
sensorManager.addSecondarySensor(sensor);
}| for (const sensorName of sensorNames) { | ||
| const sensor = sensorManager.getSensorByObjName(sensorName); | ||
|
|
||
| if (sensor) { | ||
| sensorManager.addSecondarySensor(sensor); | ||
| } else { | ||
| errorManagerInstance.warn(`Sensor ${sensorName} not found in catalog`); | ||
| } | ||
| } | ||
|
|
||
| if (sensorNames.length > 0) { | ||
| ServiceLocator.getUiManager().toast( | ||
| `Loaded ${sensorNames.length} sensor(s)`, |
There was a problem hiding this comment.
The toast message shows sensorNames.length sensors were loaded, but this count includes sensors that may not have been found in the catalog (since getSensorByObjName will return null for sensors that aren't currently active, which they won't be after clearing). Additionally, the condition if (sensorNames.length > 0) should check the actual number of successfully loaded sensors. Consider tracking the actual count of successfully loaded sensors and displaying that instead.
| for (const sensorName of sensorNames) { | |
| const sensor = sensorManager.getSensorByObjName(sensorName); | |
| if (sensor) { | |
| sensorManager.addSecondarySensor(sensor); | |
| } else { | |
| errorManagerInstance.warn(`Sensor ${sensorName} not found in catalog`); | |
| } | |
| } | |
| if (sensorNames.length > 0) { | |
| ServiceLocator.getUiManager().toast( | |
| `Loaded ${sensorNames.length} sensor(s)`, | |
| let loadedCount = 0; | |
| for (const sensorName of sensorNames) { | |
| const sensor = sensorManager.getSensorByObjName(sensorName); | |
| if (sensor) { | |
| sensorManager.addSecondarySensor(sensor); | |
| loadedCount++; | |
| } else { | |
| errorManagerInstance.warn(`Sensor ${sensorName} not found in catalog`); | |
| } | |
| } | |
| if (loadedCount > 0) { | |
| ServiceLocator.getUiManager().toast( | |
| `Loaded ${loadedCount} sensor(s)`, |
| const sensors: string[] = []; | ||
|
|
||
| // Collect all current sensors | ||
| for (const sensor of sensorManager.currentSensors) { | ||
| if (sensor?.objName) { | ||
| sensors.push(sensor.objName); | ||
| } | ||
| } | ||
|
|
||
| // Collect all secondary sensors | ||
| for (const sensor of sensorManager.secondarySensors) { | ||
| if (sensor?.objName) { | ||
| sensors.push(sensor.objName); | ||
| } | ||
| } | ||
|
|
||
| return sensors; |
There was a problem hiding this comment.
This method may include duplicate sensors if a sensor appears in both currentSensors and secondarySensors. Consider using a Set to ensure uniqueness:
private collectActiveSensors_(): string[] {
const sensorManager = ServiceLocator.getSensorManager();
const sensorNames = new Set<string>();
// Collect all current sensors
for (const sensor of sensorManager.currentSensors) {
if (sensor?.objName) {
sensorNames.add(sensor.objName);
}
}
// Collect all secondary sensors
for (const sensor of sensorManager.secondarySensors) {
if (sensor?.objName) {
sensorNames.add(sensor.objName);
}
}
return Array.from(sensorNames);
}| const sensors: string[] = []; | |
| // Collect all current sensors | |
| for (const sensor of sensorManager.currentSensors) { | |
| if (sensor?.objName) { | |
| sensors.push(sensor.objName); | |
| } | |
| } | |
| // Collect all secondary sensors | |
| for (const sensor of sensorManager.secondarySensors) { | |
| if (sensor?.objName) { | |
| sensors.push(sensor.objName); | |
| } | |
| } | |
| return sensors; | |
| const sensorNames = new Set<string>(); | |
| // Collect all current sensors | |
| for (const sensor of sensorManager.currentSensors) { | |
| if (sensor?.objName) { | |
| sensorNames.add(sensor.objName); | |
| } | |
| } | |
| // Collect all secondary sensors | |
| for (const sensor of sensorManager.secondarySensors) { | |
| if (sensor?.objName) { | |
| sensorNames.add(sensor.objName); | |
| } | |
| } | |
| return Array.from(sensorNames); |
|
…tering
This implements STK-style scenario management where scenario files contain lists of satellites and sensors that should be visible when loaded.