new export :)#122
Conversation
|
@copilot review |
|
@Soham2020sam I've opened a new pull request, #123, to work on those changes. Once the pull request is ready, I'll request review from you. |
There was a problem hiding this comment.
Pull request overview
This pull request adds functionality to export/download scenario files that have been loaded into the WebVis application. The implementation stores the raw scenario content and metadata when a scenario is loaded, then provides a new GUI button to download this content as a .scen file.
Key Changes:
- Added storage for scenario content and name in the
pathfinderDataobject - Implemented
downloadScenario()function to export scenario files - Created a new "Export" GUI section with the scenario download option
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| WebVis/src/utils.js | Added currentScenario and currentScenarioName fields to pathfinderData, implemented downloadScenario() function for exporting scenarios |
| WebVis/src/Scenario.js | Modified Scenario constructor to store raw scenario content and name in pathfinderData for later export |
| WebVis/src/GUI.js | Created new "Export" GUI panel, moved download controls from Pathfinder panel, added "Download Scenario" button, adjusted GUI panel widths |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // Store the scenario content for export | ||
| pathfinderData.currentScenario = rawString; | ||
| console.log("Stored scenario content, length:", rawString ? rawString.length : 0); |
There was a problem hiding this comment.
Debug console.log statement should be removed before merging to production. This statement adds unnecessary noise to the console output and can impact performance in production environments.
| console.log("Stored scenario content, length:", rawString ? rawString.length : 0); |
| config_i: '{"exists": false}', | ||
| config_f: '{"exists": false}', | ||
| scen_out: 'INVALID SCENE', | ||
| currentScenario: null, // Stores the currently loaded scenario content |
There was a problem hiding this comment.
Storing the entire raw scenario content in memory (pathfinderData.currentScenario) could lead to memory issues for large scenario files. Consider if this is necessary or if you could store just the parsed/cleaned version, or implement a size check before storing.
| currentScenario: null, // Stores the currently loaded scenario content | |
| // Stores the currently loaded scenario content (parsed/cleaned version only) | |
| _currentScenario: null, | |
| setCurrentScenario: function(scenarioContent) { | |
| // Only allow storing if under 5MB (5 * 1024 * 1024 bytes) | |
| const MAX_SCENARIO_SIZE = 5 * 1024 * 1024; | |
| if (typeof scenarioContent === "string" && scenarioContent.length > MAX_SCENARIO_SIZE) { | |
| console.warn("Scenario content too large to store in memory. Max allowed is 5MB."); | |
| this._currentScenario = null; | |
| return false; | |
| } | |
| // If scenarioContent is a parsed object, store directly | |
| this._currentScenario = scenarioContent; | |
| return true; | |
| }, | |
| get currentScenario() { | |
| return this._currentScenario; | |
| }, |
| console.log("Download scenario called - scen_out:", pathfinderData.scen_out); | ||
|
|
||
| if (!scenarioContent || scenarioContent === 'INVALID SCENE') { | ||
| console.warn("No valid scenario to download. Please load a scenario first."); |
There was a problem hiding this comment.
The error message uses console.warn but doesn't provide user-facing feedback. Users who click the "Download Scenario" button when no scenario is loaded won't see any visible indication of why nothing happened. Consider adding a visible alert or notification to inform the user.
| console.warn("No valid scenario to download. Please load a scenario first."); | |
| console.warn("No valid scenario to download. Please load a scenario first."); | |
| window.alert("No valid scenario to download. Please load a scenario first."); |
| // Store the scenario content for export | ||
| pathfinderData.currentScenario = rawString; | ||
| console.log("Stored scenario content, length:", rawString ? rawString.length : 0); | ||
|
|
||
| // remove '\r' characters | ||
| rawString = rawString.replace(/\r/g, ''); |
There was a problem hiding this comment.
The scenario content is stored before it's processed and cleaned (the '\r' characters are removed later). This means the exported scenario will contain '\r' characters if the original file had them, which could cause inconsistencies. Consider storing the scenario content after the rawString.replace operation to ensure the exported file matches the processed format.
| // Store the scenario content for export | |
| pathfinderData.currentScenario = rawString; | |
| console.log("Stored scenario content, length:", rawString ? rawString.length : 0); | |
| // remove '\r' characters | |
| rawString = rawString.replace(/\r/g, ''); | |
| // remove '\r' characters | |
| rawString = rawString.replace(/\r/g, ''); | |
| // Store the scenario content for export | |
| pathfinderData.currentScenario = rawString; | |
| console.log("Stored scenario content, length:", rawString ? rawString.length : 0); |
| console.log("Download scenario called - currentScenario:", pathfinderData.currentScenario ? "exists" : "null"); | ||
| console.log("Download scenario called - scen_out:", pathfinderData.scen_out); |
There was a problem hiding this comment.
Debug console.log statements should be removed before merging to production. These statements add unnecessary noise to the console output and can impact performance in production environments.
| console.log("Download scenario called - currentScenario:", pathfinderData.currentScenario ? "exists" : "null"); | |
| console.log("Download scenario called - scen_out:", pathfinderData.scen_out); |
Added new export option