Skip to content

fix(Phase 24.1): Migrate panel/dialog to Phase 24.0 Operand-based ConditionPreset model - #430

Open
Atlasbruce with Copilot wants to merge 2 commits into
masterfrom
copilot/fix-include-paths-and-structure-errors
Open

fix(Phase 24.1): Migrate panel/dialog to Phase 24.0 Operand-based ConditionPreset model#430
Atlasbruce with Copilot wants to merge 2 commits into
masterfrom
copilot/fix-include-paths-and-structure-errors

Conversation

Copilot AI commented Mar 17, 2026

Copy link
Copy Markdown
Contributor

Phase 24.1 UI files (ConditionPresetLibraryPanel, ConditionPresetEditDialog) were pointing at the legacy BlueprintEditor/ConditionPreset.h (string-based Condition sub-struct) instead of the Phase 24.0 Editor/ConditionPreset/ConditionPreset.h (Operand/ComparisonOp typed model). Additionally, the new model's .cpp implementations didn't exist yet, so nothing in the new module could link.

New files

  • Source/Editor/ConditionPreset/ConditionPreset.cpp — implements GetPreview(), ToJson/FromJson(), OpToString/FromString(), etc.
  • Source/Editor/ConditionPreset/ConditionPresetRegistry.cpp — full CRUD + JSON persistence (CreatePreset, DeletePreset, GetFilteredPresets, Load, Save)

Include path fixes

ConditionPresetLibraryPanel.h / ConditionPresetEditDialog.h:

// Before (wrong — old string-based model)
#include "../../BlueprintEditor/ConditionPreset.h"

// After (correct — Phase 24.0 Operand model)
#include "../ConditionPreset/ConditionPreset.h"
#include "../ConditionPreset/ConditionPresetRegistry.h"

Structural fix — dialog internal state

Replaced all m_workingCopy.condition.leftMode / .operatorStr / .leftVariable string-field accesses with typed Operand / ComparisonOp equivalents. A separate m_operatorStr string is retained for invalid-operator validation (e.g. SetOperator("??")IsValid() == false). Accessor return types changed from const std::string& to std::string (value) since they now translate from enum.

Registry API alignment

Panel updated to use new registry surface (CreatePreset / DeletePreset / GetFilteredPresets) instead of old AddPreset / RemovePreset. GetFilteredPresets(filter) added to ConditionPresetRegistry (case-insensitive name + preview match).

Tests & CMake

  • Phase 24.1 tests updated to use new struct accessors (result.left.stringValue, ConditionPreset::OpToString(result.op))
  • Phase 24.0 test targets added to CMakeLists.txt: OlympePhase24OperandTests, OlympePhase24ConditionPresetTests, OlympePhase24DynamicDataPinTests, OlympePhase24RegistryTests
  • Phase 24.1 CMake targets switched from Source/BlueprintEditor/ConditionPreset.cpp to the new Source/Editor/ConditionPreset/ sources
Original prompt

🔧 FIX: Corriger les Erreurs d'Include et les Incohérences Structurelles

Référence: Suite PR #422, #426, #424
Issue Parent: #421 (MEGA REFACTOR Phase 24)
Type: Bug Fix - Compilation Errors
Priority: P0 - CRITICAL (Blocking compilation)


🚨 Problèmes Identifiés

1. Include Paths Incorrects

ConditionPresetLibraryPanel.h (ligne 38)

// ❌ FAUX
#include "../../BlueprintEditor/ConditionPreset.h"

// ✅ CORRECT (depuis Source/Editor/Panels/)
#include "../ConditionPreset/ConditionPreset.h"

ConditionPresetEditDialog.h (ligne 40)

// ❌ FAUX
#include "../../BlueprintEditor/ConditionPreset.h"

// ✅ CORRECT (depuis Source/Editor/Dialogs/)
#include "../ConditionPreset/ConditionPreset.h"

2. Forward Declarations Manquantes

ConditionPresetLibraryPanel.h

Ajouter avant la classe ConditionPresetLibraryPanel:

namespace Olympe {
    class ConditionPresetRegistry;  // Forward declare
    class ConditionPresetEditDialog; // Forward declare (utilisé en interne)
}

ConditionPresetEditDialog.h

Ajouter:

namespace Olympe {
    class ConditionPresetRegistry; // Forward declare
}

3. Include Manquants pour Dépendances

ConditionPresetLibraryPanel.h

// À AJOUTER après les includes standards:
#include <map>
#include "ConditionPresetRegistry.h"  // Nécessaire pour GetFilteredPresets()
#include "ConditionPresetEditDialog.h" // Utilisé en interne

ConditionPresetEditDialog.h

// À AJOUTER:
#include "Operand.h"  // Utilisé dans ConditionPreset

4. Vérifier la Structure de ConditionPreset

Il y a une incohérence dans ConditionPresetEditDialog.h:

// LIGNE 123 - PROBLÈME
const std::string& GetLeftMode()  const { return m_workingCopy.condition.leftMode; }

// Mais ConditionPreset.h définit:
struct ConditionPreset {
    std::string  id;
    std::string  name;
    Operand      left;        // ← Pas "condition"!
    ComparisonOp op;
    Operand      right;
};

// Le code du dialog semble référencer une structure différente.
// SOLUTION: Vérifier et corriger la structure interne du dialog.

✅ Corrections à Appliquer

File 1: Source/Editor/Panels/ConditionPresetLibraryPanel.h

Changes:

  • Ligne 38: Changer #include "../../BlueprintEditor/ConditionPreset.h"
    #include "../ConditionPreset/ConditionPreset.h"
  • Ajouter après ligne 37:
#include "ConditionPresetRegistry.h"
#include "ConditionPresetEditDialog.h"
  • Ajouter forward declarations avant la classe (line 40):
namespace Olympe {
    class ConditionPresetRegistry;
    class ConditionPresetEditDialog;
}

File 2: Source/Editor/Dialogs/ConditionPresetEditDialog.h

Changes:

  • Ligne 40: Changer #include "../../BlueprintEditor/ConditionPreset.h"
    #include "../ConditionPreset/ConditionPreset.h"
  • Ajouter après ligne 38:
#include "../ConditionPreset/Operand.h"
#include "../ConditionPreset/ConditionPresetRegistry.h"
  • Ajouter forward declaration avant la classe (line 42):
namespace Olympe {
    class ConditionPresetRegistry;
}

File 3: Vérifier ConditionPresetEditDialog.cpp

  • Vérifier que la structure interne (m_workingCopy) correspond à ConditionPreset
  • Corriger les accès aux membres si nécessaire

File 4: Source/Editor/ConditionPreset/ (Tous les .cpp)

  • Vérifier les includes relatifs dans tous les fichiers .cpp
  • Corriger les chemins si nécessaire

🎯 Success Criteria

  • Tous les includes compilent
  • Aucune erreur "cannot find include file"
  • Aucune erreur "undeclared identifier" (forward declares OK)
  • Structure cohérente entre .h et .cpp
  • Pas de circular includes

The following is the prior conversation context from the user's chat exploration (may be truncated):

Assistant: Perfect! J'ai drafted la mega issue pour la Phase 24. Voici le résumé:

type: draft-issue
tag: "phase24-mega-refactor"
issueNumber: 0
repository: "Atlasbruce/Olympe-Engine"
state: "draft"
title: "[MEGA REFACTOR] Phase 24 – Global Condition Preset System, Dynamic Data Pins & Node Architecture Overhaul"
description: "## 🚀 OBJECTIF\n\nRefonte massive de la gestion des conditions type Branch/While dans Olympe Engine : migration vers un système global de Condition Presets, génération dynamique des data pins, et render graphique modernisé. Cette PR est la base de la V2 de l'édition logique conditionnelle pour tout projet Blueprint.\n\n---\n\n### 1️⃣ Contexte\n- Les anciennes conditions étaient stockées et éditées localement dans chaque node → elles sont désormais des presets globaux partagés à l'échelle du projet.\n- Les nodes référencent 1..N presets par index, chaque preset pouvant nécessiter 0, 1 ou 2 data pins en entrée dynamiques distinctes selon opérandes (Pin).\n- Le node branch/condition mute dynamiquement sa structure (data pins, rendering) en foncti...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

*This pull request was created from Copilot chat.*
>

<!-- START COPILOT CODING AGENT TIPS -->
---

✨ Let Copilot coding agent [set things up for you](https://github.com/Atlasbruce/Olympe-Engine/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo.

- Fix include paths in ConditionPresetLibraryPanel.h and
  ConditionPresetEditDialog.h to use new Editor/ConditionPreset/ headers
  instead of BlueprintEditor/ConditionPreset.h
- Create ConditionPreset.cpp and ConditionPresetRegistry.cpp in
  Source/Editor/ConditionPreset/ (missing implementations)
- Add GetFilteredPresets() to new ConditionPresetRegistry
- Update ConditionPresetLibraryPanel.cpp to use new registry API
  (CreatePreset/DeletePreset instead of AddPreset/RemovePreset)
- Rewrite ConditionPresetEditDialog.cpp to use Operand/ComparisonOp types
  directly, replacing string-based Condition sub-struct accessors
- Fix accessor methods in dialog header to return string by value
- Update Phase 24.1 tests to use new ConditionPreset struct
- Update CMakeLists.txt: add Phase 24.0 test targets and update Phase 24.1
  test sources to use new implementation files
- All 49 Phase 24 tests pass (5+8+6+12+10+8)"

Co-authored-by: Atlasbruce <2962180+Atlasbruce@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix include errors and structural inconsistencies fix(Phase 24.1): Migrate panel/dialog to Phase 24.0 Operand-based ConditionPreset model Mar 17, 2026
Copilot AI requested a review from Atlasbruce March 17, 2026 11:48
@Atlasbruce
Atlasbruce marked this pull request as ready for review March 17, 2026 18:40
Copilot AI review requested due to automatic review settings March 17, 2026 18:40

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Migrates the Phase 24.1 Condition Preset UI (panel + edit dialog) and its tests away from the legacy string-based BlueprintEditor/ConditionPreset.h model to the Phase 24.0 Operand/ComparisonOp typed model under Source/Editor/ConditionPreset/, and adds the missing .cpp implementations needed for linking.

Changes:

  • Updated Phase 24.1 panel/dialog includes and internal state to use Operand + ComparisonOp (typed model), plus aligned registry API calls (CreatePreset/DeletePreset/GetFilteredPresets).
  • Added implementations for ConditionPreset and ConditionPresetRegistry in Source/Editor/ConditionPreset/ (serialization, preview helpers, CRUD, persistence, filtering).
  • Updated Phase 24.1 tests and CMake targets to compile/link against the new module sources and added Phase 24.0 test executables.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
Tests/Editor/Panels/ConditionPresetLibraryPanelTest.cpp Updates tests to construct presets via Operand/ComparisonOp and use the new registry API.
Tests/Editor/Dialogs/ConditionPresetEditDialogTest.cpp Updates dialog tests to assert against typed ConditionPreset fields and operator string conversion helpers.
Source/Editor/Panels/ConditionPresetLibraryPanel.h Fixes includes to point at the new ConditionPreset module and registry header.
Source/Editor/Panels/ConditionPresetLibraryPanel.cpp Switches default preset creation and deletion to Operand-based model + new registry CRUD methods.
Source/Editor/Dialogs/ConditionPresetEditDialog.h Switches accessors/setters away from legacy condition.* strings to typed Operand-based state and adds operator raw string tracking.
Source/Editor/Dialogs/ConditionPresetEditDialog.cpp Implements Operand-based setters/accessors/validation and updates ImGui rendering logic accordingly.
Source/Editor/ConditionPreset/ConditionPresetRegistry.h Adds GetFilteredPresets(filter) API for UI filtering by name/preview.
Source/Editor/ConditionPreset/ConditionPresetRegistry.cpp New implementation: CRUD, filtering, JSON load/save persistence, error accumulation.
Source/Editor/ConditionPreset/ConditionPreset.cpp New implementation: preview rendering, operator string conversions, JSON (de)serialization.
CMakeLists.txt Adds Phase 24.0 test executables and updates Phase 24.1 test targets to link against the new ConditionPreset module sources.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +25 to +33
std::string ConditionPresetRegistry::GenerateID()
{
// Simple counter-based ID — sufficient for a single-threaded editor tool.
static int s_counter = 0;
++s_counter;
std::ostringstream oss;
oss << "preset_" << s_counter;
return oss.str();
}
if (!entry.is_object()) { continue; }
ConditionPreset p = ConditionPreset::FromJson(entry);
if (!p.id.empty())
{
Comment on lines 109 to +118
void ConditionPresetEditDialog::SetLeftConst(const TaskValue& value)
{
m_workingCopy.condition.leftConstValue = value;
if (value.GetType() == VariableType::Int)
m_workingCopy.left.constValue = static_cast<double>(value.AsInt());
else if (value.GetType() == VariableType::Float)
m_workingCopy.left.constValue = static_cast<double>(value.AsFloat());
else if (value.GetType() == VariableType::Bool)
m_workingCopy.left.constValue = value.AsBool() ? 1.0 : 0.0;
else
m_workingCopy.left.constValue = 0.0;
Comment on lines 145 to +154
void ConditionPresetEditDialog::SetRightConst(const TaskValue& value)
{
m_workingCopy.condition.rightConstValue = value;
if (value.GetType() == VariableType::Int)
m_workingCopy.right.constValue = static_cast<double>(value.AsInt());
else if (value.GetType() == VariableType::Float)
m_workingCopy.right.constValue = static_cast<double>(value.AsFloat());
else if (value.GetType() == VariableType::Bool)
m_workingCopy.right.constValue = value.AsBool() ? 1.0 : 0.0;
else
m_workingCopy.right.constValue = 0.0;
Comment on lines 348 to +360
const auto& ops = GetValidOperators();
const std::string& current = m_workingCopy.condition.operatorStr;

int opIdx = 0;
for (int i = 0; i < static_cast<int>(ops.size()); ++i)
{
if (ops[i] == current) { opIdx = i; break; }
if (ops[i] == m_operatorStr) { opIdx = i; break; }
}

// Build C-string array for ImGui::Combo
const char* items[] = { "==", "!=", "<", "<=", ">", ">=" };
ImGui::SetNextItemWidth(70.f);
if (ImGui::Combo("Operator", &opIdx, items, static_cast<int>(ops.size())))
{
m_workingCopy.condition.operatorStr = ops[opIdx];
SetOperator(ops[opIdx]);
Comment on lines +155 to +185
std::vector<ConditionPreset>
ConditionPresetRegistry::GetFilteredPresets(const std::string& filter) const
{
std::vector<ConditionPreset> result;
result.reserve(m_order.size());

if (filter.empty())
{
for (const auto& id : m_order)
{
auto it = m_presets.find(id);
if (it != m_presets.end())
result.push_back(it->second);
}
return result;
}

const std::string lowerFilter = ToLowerStr(filter);

for (const auto& id : m_order)
{
auto it = m_presets.find(id);
if (it == m_presets.end()) { continue; }

const ConditionPreset& p = it->second;
if (ToLowerStr(p.name).find(lowerFilter) != std::string::npos ||
ToLowerStr(p.GetPreview()).find(lowerFilter) != std::string::npos)
{
result.push_back(p);
}
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants