feat(isl): add export changes to json function #1540
Conversation
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
WalkthroughAdds JSON export capabilities to LightEditor: new DrawSettings buttons to export all lights or the selected light; implements ExportLightsToJson, ExportSelectedLightToJson, CreateLightJsonData, and PopulateLightRuntimeData; writes timestamped files into LightExports and includes runtime/editor context and _islMetadata. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as LightEditor.DrawSettings
participant LE as LightEditor
participant FS as Filesystem
participant Log as Logger
User->>UI: Click "Export All Lights to JSON"
UI->>LE: ExportLightsToJson()
rect rgba(230,245,255,0.5)
LE->>LE: Gather lights\nPopulateLightRuntimeData per light\nCreateLightJsonData per light\nGroup by model key (Reference/Attached)
note right of LE #ccf: Builds Lights array with _islMetadata, TES flags, offsets, editor/runtime context
end
LE->>FS: Write LightExports/<timestamp>_all.json
alt success
FS-->>LE: OK
LE->>Log: Info: export success
else failure
FS-->>LE: Error
LE->>Log: Error: export failed
end
User->>UI: Click "Export Selected Light"
UI->>LE: ExportSelectedLightToJson()
rect rgba(240,255,240,0.5)
LE->>LE: PopulateLightRuntimeData(selected)\nCreateLightJsonData(selected)
note right of LE #cfc: Includes selection context and metadata
end
LE->>FS: Write LightExports/<timestamp>_selected.json
alt success
FS-->>LE: OK
LE->>Log: Info: export success
else failure
FS-->>LE: Error
LE->>Log: Error: export failed
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
…11/skyrim-community-shaders into Light-editor-jsons-export
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Features/InverseSquareLighting/LightEditor.cpp(3 hunks)src/Features/InverseSquareLighting/LightEditor.h(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{cpp,cxx,cc,c,h,hpp,hxx,hlsl,hlsli,fx,fxh,py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not include TODO/FIXME placeholders; provide complete, working solutions
Files:
src/Features/InverseSquareLighting/LightEditor.cppsrc/Features/InverseSquareLighting/LightEditor.h
src/**/*.{cpp,cxx,cc,h,hpp,hxx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{cpp,cxx,cc,h,hpp,hxx}: Ensure SE/AE/VR runtime compatibility; use runtime detection patterns (e.g., REL::RelocateMember())
Include robust error handling and resource management with graceful degradation in the plugin code
Files:
src/Features/InverseSquareLighting/LightEditor.cppsrc/Features/InverseSquareLighting/LightEditor.h
🧬 Code graph analysis (1)
src/Features/InverseSquareLighting/LightEditor.h (1)
src/Features/InverseSquareLighting/LightEditor.cpp (6)
ExportLightsToJson(363-456)ExportLightsToJson(363-363)ExportSelectedLightToJson(567-654)ExportSelectedLightToJson(567-567)CreateLightJsonData(458-565)CreateLightJsonData(458-458)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build plugin and addons
|
✅ A pre-release build is available for this PR: |
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
|
@coderabbitai do another review and make sure the export function works as intended. |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Features/InverseSquareLighting/LightEditor.cpp(5 hunks)src/Features/InverseSquareLighting/LightEditor.h(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Features/InverseSquareLighting/LightEditor.h
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{cpp,cxx,cc,c,h,hpp,hxx,hlsl,hlsli,fx,fxh,py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not include TODO/FIXME placeholders; provide complete, working solutions
Files:
src/Features/InverseSquareLighting/LightEditor.cpp
src/**/*.{cpp,cxx,cc,h,hpp,hxx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{cpp,cxx,cc,h,hpp,hxx}: Ensure SE/AE/VR runtime compatibility; use runtime detection patterns (e.g., REL::RelocateMember())
Include robust error handling and resource management with graceful degradation in the plugin code
Files:
src/Features/InverseSquareLighting/LightEditor.cpp
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build plugin and addons
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Features/InverseSquareLighting/LightEditor.cpp (1)
418-429: Consider logging the count of filtered "Other" lights.The export silently excludes lights where neither
isRefnorisAttachedis true. Users won't know how many lights were skipped.Apply this diff to inform users:
int metadataLightCount = 0; + int skippedOtherCount = 0; for (const auto& light : lights) { // Only export lights that have metadata (isRef or isAttached) if (light.isRef || light.isAttached) { // Group by type for organizational purposes // Note: Using placeholder keys, not actual base object model paths // This is intentional as not all lights have associated .nif models std::string modelKey = fmt::format("ISL_Export_Group_{}", light.isRef ? "Reference" : "Attached"); lightsByModel[modelKey].push_back(&light); metadataLightCount++; + } else { + skippedOtherCount++; } } + + if (skippedOtherCount > 0) { + logger::info("Skipped {} 'Other' lights (lights without reference metadata)", skippedOtherCount); + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Features/InverseSquareLighting/LightEditor.cpp(7 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{cpp,cxx,cc,c,h,hpp,hxx,hlsl,hlsli,fx,fxh,py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not include TODO/FIXME placeholders; provide complete, working solutions
Files:
src/Features/InverseSquareLighting/LightEditor.cpp
src/**/*.{cpp,cxx,cc,h,hpp,hxx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{cpp,cxx,cc,h,hpp,hxx}: Ensure SE/AE/VR runtime compatibility; use runtime detection patterns (e.g., REL::RelocateMember())
Include robust error handling and resource management with graceful degradation in the plugin code
Files:
src/Features/InverseSquareLighting/LightEditor.cpp
🧠 Learnings (1)
📚 Learning: 2025-06-08T11:25:14.536Z
Learnt from: alandtse
PR: doodlum/skyrim-community-shaders#577
File: src/Features/WetnessEffects.h:36-36
Timestamp: 2025-06-08T11:25:14.536Z
Learning: In the skyrim-community-shaders project, boolean flags in C++ structs that interface with HLSL shaders use `uint` type instead of `bool` for compatibility reasons. This ensures consistent 4-byte size, proper 16-byte alignment in constant buffers, and cross-platform compatibility when passing data between C++ and HLSL shaders.
Applied to files:
src/Features/InverseSquareLighting/LightEditor.cpp
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build plugin and addons
🔇 Additional comments (11)
src/Features/InverseSquareLighting/LightEditor.cpp (11)
45-58: LGTM! Clean UI integration.The export buttons are well-integrated with informative tooltips and proper action handlers.
105-112: Proper accessor usage for flag checks.Good fix using
.any()and.set()/.reset()accessors instead of unsafe reinterpret operations.
144-159: Correct use of.underlying()for ImGui flag checkboxes.Using
.underlying()to get the raw TES flag mask for ImGui integration is the proper approach, and reconstructing the enumeration wrapper afterwards maintains type safety.
566-591: Safe flag handling in JSON export.Both the InverseSquare flag check (using
.any()) and TES flag export (using.underlying()) correctly use the accessor methods.
241-242: Correct placement of runtime data population.Populating runtime data during
GatherLightsensures each light's state is captured fresh, fixing the previous critical issue where global editor state was incorrectly used for all lights.
346-372: Per-light data capture resolves previous critical issue.This function properly captures runtime data, TES flags, and metadata for each individual light, replacing the incorrect approach of reading from global editor state. The intentional use of
{0, 0, 0}offset for ref lights (which use world positions directly) is clearly documented.
404-409: LGTM! Proper validation.Early return with clear logging when no lights are available.
453-463: Good documentation of ISL-specific format limitations.The metadata warnings clearly communicate that placeholder model keys are not actual .nif paths and that the format is ISL-specific, not directly compatible with Light Placer. This transparency helps users understand the export's limitations.
477-504: Robust file I/O and JSON serialization error handling.Both directory creation and JSON serialization are wrapped in try-catch blocks with appropriate error logging.
509-621: Comprehensive light data export with proper per-light state.The function correctly:
- Uses per-light data from
lightInfoinstead of global editor state- Exports
cutoffOverridewhen non-zero (line 536-538)- Uses safe flag accessors (
.any()and.underlying())- Conditionally exports size, offset, and flags to minimize JSON clutter
- Includes rich ISL metadata for debugging and reference
These changes address the critical issues from previous reviews.
623-719: Well-structured single light export with useful context.The function properly validates selection, includes helpful metadata (player position, selected light info), and has robust error handling. The structure mirrors
ExportLightsToJsonfor consistency.
|
How many different file export formats are we going to have? |
This is designed to mostly mimic how light placer lays out files. It was highly requested from some devs as theres no way to dump the data from the editor. But i guess we will have quite a few for the different systems we have. I know this is starting to become a problem but I cannot think of a decent solution. One big json gets messy fast. |
|
Maybe sync with @sicsix , I know they purposely didn't have an output for some reason. |
Will shoot him a message |
From sic Idea seems fine, go for it. Just couldn't be bothered doing it myself, no real reason |
2 participants
adds two functions, export current selected light to json data dump, and all edited lights in this session.
Summary by CodeRabbit