Changeset 388 for cafu/trunk

Timestamp:

09/20/11 21:18:12 (8 months ago)

Author:

Carsten

Message:

Gui Editor:
Make sure that windows have valid names at all times, so that saving and re-loading the GUI works as expected.
A window name is valid if it is a valid Lua script identifier and unique among all windows in the GUI.

Fixes #87 and #89.
Many thanks to Joe for reports and analysis!

Location:

cafu/trunk/CaWE

Files:

• DialogInsp-EntityProps.cpp (modified) (1 diff)
• GuiEditor/Commands/ModifyWindow.cpp (modified) (1 diff)
• GuiEditor/Commands/Paste.cpp (modified) (1 diff)
• GuiEditor/GuiDocument.cpp (modified) (4 diffs)
• GuiEditor/GuiDocument.hpp (modified) (1 diff)
• GuiEditor/WindowTree.cpp (modified) (1 diff)
• GuiEditor/Windows/EditorWindow.cpp (modified) (2 diffs)
• GuiEditor/Windows/EditorWindow.hpp (modified) (2 diffs)
• LuaAux.cpp (modified) (1 diff)
• LuaAux.hpp (modified) (1 diff)
• MapEntity.cpp (modified) (1 diff)
• ModelEditor/ObserverPattern.hpp (modified) (1 diff)

cafu/trunk/CaWE/DialogInsp-EntityProps.cpp

@@ -949 +949 @@
 
         // Check Lua compatibility for entity names.
-        if (Key=="name" && !CheckLuaVarCompat(NewValue))
-        {
-            wxMessageBox("Entity names must be valid Lua identifiers: They can be any combination of letters, digits and underscores that does not begin with a digit and is not a reserved Lua keyword.", "Error: Entity name is not a Lua identifier.", wxOK | wxICON_ERROR);
+        if (Key=="name" && !IsLuaIdentifier(NewValue))
+        {
+            wxMessageBox("An entity name must be a string of letters, digits, and underscores that is\n"
+                "not beginning with a digit and is not a reserved Lua keyword or global variable.",
+                "Entity name is not a valid script identifier.", wxOK | wxICON_ERROR);
+
             NotifySubjectChanged_Modified(MapDoc, MapElements, MEMD_ENTITY_PROPERTY_MODIFIED, ""); // Intentionally update also this dialog to restore previous property value.
             return;

cafu/trunk/CaWE/GuiEditor/Commands/ModifyWindow.cpp

@@ -115 +115 @@
     {
         m_OldString=m_Window->Name;
-
-        if (!GuiDocumentT::GetSibling(m_Window)->SetName(m_NewString))
-        {
-            m_GuiDocument->UpdateAllObservers_Modified(m_Window, WMD_PROPERTY_CHANGED, m_PropertyName); // This is needed to reset the property grid correctly.
-            return false;
-        }
+        m_Window->Name=m_GuiDocument->CheckWindowName(m_NewString, GuiDocumentT::GetSibling(m_Window));
     }
     else if (m_PropertyName=="BackMatName")

cafu/trunk/CaWE/GuiEditor/Commands/Paste.cpp

@@ -86 +86 @@
         m_Windows[WinNr]->Parent=m_NewParent;
         m_NewParent->Children.PushBack(m_Windows[WinNr]);
-
-        // If the name of the window is not unique among its siblings, find a new unique name.
-        GuiDocumentT::GetSibling(m_Windows[WinNr])->RepairNameUniqueness();
     }
 

cafu/trunk/CaWE/GuiEditor/GuiDocument.cpp

@@ -24 +24 @@
 #include "../GameConfig.hpp"
 #include "../EditorMaterialEngine.hpp"
+#include "../LuaAux.hpp"
 
 #include "GuiSys/GuiImpl.hpp"
@@ -76 +77 @@
     else
     {
-        m_Gui=new cf::GuiSys::GuiImplT("Win=gui:new('WindowT'); gui:SetRootWindow(Win); gui:showMouse(false); gui:setFocus(Win); Win:SetName('Window'); Win:set(\"rect\", 0, 0, 640, 480);", true);
+        m_Gui=new cf::GuiSys::GuiImplT("Win=gui:new('WindowT'); gui:SetRootWindow(Win); gui:showMouse(false); gui:setFocus(Win); Win:SetName('Root'); Win:set(\"rect\", 0, 0, 640, 480);", true);
 
         m_GuiProperties=GuiPropertiesT(*m_Gui);
@@ -139 +140 @@
 
 
-const ArrayT<cf::GuiSys::WindowT*>& GuiDocumentT::GetSelection()
-{
-    return m_Selection;
-}
-
-
-// Recursively makes sure that the children of each window have unique names.
-// Normally the other GUI editor code should make sure that that is always true, but right now it
-// is possible to create violations of this constraint via drag-and-drop in the window hierarchy tree
-// (can drop a window as a child of another window that already has a child with the same name).
-static void CheckWindowNames(cf::GuiSys::WindowT* Window)
-{
-    const std::string OldName=Window->Name;
-
-    GuiDocumentT::GetSibling(Window)->RepairNameUniqueness();
-
-    if (Window->Name!=OldName)
-        GuiDocumentT::GetSibling(Window)->GetGuiDoc()->UpdateAllObservers_Modified(Window, WMD_PROPERTY_CHANGED, "Name");
-
-    for (unsigned long ChildNr=0; ChildNr<Window->Children.Size(); ChildNr++)
-        CheckWindowNames(Window->Children[ChildNr]);
+wxString GuiDocumentT::CheckWindowName(const wxString& TestName, EditorWindowT* Win) const
+{
+    const wxString               Name_  =CheckLuaIdentifier(TestName);
+    const cf::GuiSys::WindowT*   Win_   =Win ? Win->GetDual() : NULL;
+    wxString                     NewName=Name_;
+    ArrayT<cf::GuiSys::WindowT*> AllChildren;
+
+    AllChildren.PushBack(m_RootWindow);
+    m_RootWindow->GetChildren(AllChildren, true);
+
+    while (true)
+    {
+        bool IsUnique=true;
+
+        for (unsigned long ChildNr=0; ChildNr<AllChildren.Size(); ChildNr++)
+        {
+            if (AllChildren[ChildNr]->Name==NewName && AllChildren[ChildNr]!=Win_)
+            {
+                IsUnique=false;
+                break;
+            }
+        }
+
+        if (IsUnique) break;
+
+        static unsigned int Count=1;
+        NewName=Name_+wxString::Format("_%u", Count);
+        Count++;
+    }
+
+    return NewName;
 }
 
@@ -243 +254 @@
 bool GuiDocumentT::SaveInit_cgui(std::ostream& OutFile)
 {
-    CheckWindowNames(m_RootWindow);
-
     OutFile << "-- This is a Cafu engine GUI script file, written by CaWE, the Cafu World Editor.\n";
     OutFile << "-- You CAN edit this file manually, but note that CaWE may overwrite your changes.\n";

cafu/trunk/CaWE/GuiEditor/GuiDocument.hpp

@@ -63 +63 @@
 
         void SetSelection(const ArrayT<cf::GuiSys::WindowT*>& NewSelection);
-        const ArrayT<cf::GuiSys::WindowT*>& GetSelection();
+        const ArrayT<cf::GuiSys::WindowT*>& GetSelection() const { return m_Selection; }
 
         const ArrayT<EditorMaterialI*>& GetEditorMaterials() const { return m_EditorMaterials; }
         GameConfigT* GetGameConfig() { return m_GameConfig; }
+
+        /// Checks if the given string is a valid name for the given window.
+        /// A name is valid if it is a valid Lua identifier and unique among all windows in this GUI.
+        /// @returns
+        ///     If the given string \c TestName is valid, it is returned unchanged.
+        ///     Otherwise, a new string is created from \c TestName that is valid.
+        wxString CheckWindowName(const wxString& TestName, EditorWindowT* Win) const;
 
         bool SaveInit_cgui(std::ostream& OutFile);

cafu/trunk/CaWE/GuiEditor/WindowTree.cpp

@@ -417 +417 @@
     }
 
+    // The command may well have set a name different from TE.GetLabel().
+    TE.Veto();
+    SetItemText(TE.GetItem(), Window->Name);
+
     m_IsRecursiveSelfNotify=false;
 }

cafu/trunk/CaWE/GuiEditor/Windows/EditorWindow.cpp

@@ -25 +25 @@
 #include "../Commands/ModifyWindow.hpp"
 #include "../../EditorMaterial.hpp"
-#include "../../LuaAux.hpp"
 #include "../../MaterialBrowser/DocAccess.hpp"
 #include "../../MaterialBrowser/MaterialBrowserDialog.hpp"
@@ -46 +45 @@
     : m_Win(Win),
       m_GuiDoc(GuiDoc),
-      m_IsSelected(false),
-      m_Counter(1)
-{
-    // If window has no name, create default name.
-    if (m_Win->Name=="") m_Win->Name="Window";
-
-    // Check window name uniqueness and repair it.
-    RepairNameUniqueness();
-
-    // Note: Since the name of a window comes from an already functional script or is checked when set
-    // by the method below, we assert that the name is already Lua compatible here.
-    wxASSERT(CheckLuaVarCompat(m_Win->Name));
-}
-
-
-bool EditorWindowT::SetName(const wxString& NewName)
-{
-    if (!CheckLuaVarCompat(NewName))
-    {
-        wxMessageBox("A window name must be a string of letters, digits, and underscores that is\n"
-            "not beginning with a digit and is not a reserved Lua keyword or global variable.",
-            "Window name is not a valid Lua identifier.", wxOK | wxICON_ERROR);
-        return false;
-    }
-
-    if (!CheckNameUniqueness(NewName))
-    {
-        wxMessageBox("The window name must be unique in this window hierarchy level. The window can't have a name that is already taken by one of its siblings.", "Error: The given window name is not unique.", wxOK | wxICON_ERROR);
-        return false;
-    }
-
-    m_Win->Name=NewName.c_str();
-
-    return true;
-}
-
-
-bool EditorWindowT::CheckNameUniqueness(const wxString& Name) const
-{
-    if (m_Win->GetRoot()==m_Win) return true; // Root window can have any name.
-
-    // Get the siblings of this windows and check name uniqueness against them.
-    cf::GuiSys::WindowT* Parent=m_Win->GetParent();
-    ArrayT<cf::GuiSys::WindowT*> Siblings;
-    Parent->GetChildren(Siblings);
-
-    for (unsigned long SibNr=0; SibNr<Siblings.Size(); SibNr++)
-    {
-        if (Siblings[SibNr]==m_Win) continue; // Don't check against ourselves.
-
-        if (Siblings[SibNr]->Name==Name) return false;
-    }
-
-    return true;
-}
-
-
-static wxString StripSuffix(const wxString& Str)
-{
-    const size_t Pos=Str.rfind("_");
-
-    if (Pos==std::string::npos) return Str;
-
-    for (size_t i=Pos+1; i<Str.length(); i++)
-        if (!wxIsdigit(Str[i])) return Str;
-
-    if (Pos==0) return "Window";
-
-    return Str.Left(Pos);
-}
-
-
-void EditorWindowT::RepairNameUniqueness()
-{
-    const wxString BaseName=StripSuffix(m_Win->Name);
-    wxString       NewName =m_Win->Name;
-
-    while (!CheckNameUniqueness(NewName))
-    {
-        NewName=BaseName+"_"+wxString::Format("%u", m_Counter);
-        m_Counter++;
-    }
-
-    m_Win->Name=NewName;
+      m_IsSelected(false)
+{
+    if (m_Win->Name=="")
+    {
+        m_Win->Name=m_Win->GetType()->ClassName;
+
+        const size_t len=m_Win->Name.length();
+
+        if (len>1 && m_Win->Name[len-1]=='T')
+        {
+            // Remove the trailing "T" from our class name.
+            m_Win->Name=std::string(m_Win->Name, 0, len-1);
+        }
+    }
+
+    m_Win->Name=m_GuiDoc->CheckWindowName(m_Win->Name, this);
 }
 

cafu/trunk/CaWE/GuiEditor/Windows/EditorWindow.hpp

@@ -59 +59 @@
         bool IsSelected() const { return m_IsSelected; }
 
-        /// Sets the name for this window.
-        /// This method checks if the name is valid in the sense of Lua compatibility
-        /// (window name is used as a Lua variable name for this window) and uniqueness.
-        /// This function should always be called instead of setting the name member
-        /// of the window directly.
-        /// @param NewName   The name to be set.
-        /// @return Whether the name has been successfully set.
-        bool SetName(const wxString& NewName);
-
-        /// Checks the name uniqueness of a new name string within the windows siblings.
-        /// @param Name    Name to check for uniqueness.
-        /// @return Whether this name is unique.
-        bool CheckNameUniqueness(const wxString& Name) const;
-
-        /// Helper method to check and auto-repair the uniqueness of the name of this window.
-        void RepairNameUniqueness();
-
         /// Fills a property grid manager with one property for each class member.
         /// @param PropMan   The property manager grid to fill.
@@ -105 +88 @@
         cf::GuiSys::WindowT* m_Win;         ///< The GuiSys's "dual" or "sibling" of this window.
         GuiDocumentT*        m_GuiDoc;      ///< The GUI document that this window lives in.
-        bool                 m_IsSelected;
-        unsigned int         m_Counter;
+        bool                 m_IsSelected;  ///< Is this window selected for editing?
     };
 }

cafu/trunk/CaWE/LuaAux.cpp

@@ -26 +26 @@
 
 
-static const std::string Keywords_Lua[]={"and", "break", "do", "else", "elseif",
-                                         "end", "false", "for", "function", "if",
-                                         "in", "local", "nil", "not", "or", "repeat",
-                                         "return", "then", "true", "until", "while"};
+namespace
+{
+    const wxString Keywords[]=
+    {
+        "and", "break", "do", "else", "elseif",
+        "end", "false", "for", "function", "if",
+        "in", "local", "nil", "not", "or", "repeat",
+        "return", "then", "true", "until", "while"
+    };
 
-static const int NrOfKeywords=sizeof(Keywords_Lua)/sizeof(*Keywords_Lua);
-
-
-bool CheckLuaVarCompat(const wxString& Varname)
-{
-    wxRegEx LuaVarName("^[A-Za-z_][\\w]+$", wxRE_ADVANCED);
-    wxASSERT(LuaVarName.IsValid());
-
-    if (LuaVarName.Matches(Varname))
-    {
-        // Check if variable name is a reserved Lua keyword.
-        for (int KeywordNr=0; KeywordNr<NrOfKeywords; KeywordNr++)
-            if (Varname.c_str()==Keywords_Lua[KeywordNr]) return false;
-
-        // Check if variable name matches a Lua global variable.
-        wxRegEx LuaGlobals("^[_][A-Z]+$", wxRE_ADVANCED);
-        wxASSERT(LuaGlobals.IsValid());
-
-        if (LuaGlobals.Matches(Varname)) return false;
-
-        return true;
-    }
-
-    return false;
+    const unsigned int NrOfKeywords=sizeof(Keywords)/sizeof(*Keywords);
 }
 
 
-wxString MakeLuaVarName(const wxString& Varname)
+bool IsLuaIdentifier(const wxString& id)
 {
-    // Return the original name if it is already valid.
-    if (CheckLuaVarCompat(Varname)) return Varname;
+    // Does id fail to meet the Lua identifier rules?
+    const wxRegEx LuaIdRegEx("^[A-Za-z_][\\w]+$", wxRE_ADVANCED);
+    wxASSERT(LuaIdRegEx.IsValid());
 
-    // Return a default string if passed varname is empty so this functions never fails.
-    if (Varname=="") return "Variable";
+    if (!LuaIdRegEx.Matches(id)) return false;
 
-    wxRegEx LuaVarName("\\W", wxRE_ADVANCED);
-    wxASSERT(LuaVarName.IsValid());
+    // Is id a global reserved Lua identifier?
+    const wxRegEx LuaGlobalRegEx("^[_][A-Z]+$", wxRE_ADVANCED);
+    wxASSERT(LuaGlobalRegEx.IsValid());
 
-    wxString LuaVar=Varname;
+    if (LuaGlobalRegEx.Matches(id)) return false;
 
-    LuaVarName.Replace(&LuaVar, "_"); // Replace all non compatible chars with underscore.
+    // Is id a reserved Lua keyword?
+    for (unsigned int kwNr=0; kwNr<NrOfKeywords; kwNr++)
+        if (id==Keywords[kwNr]) return false;
 
-    // If variable is now compatible, return it.
-    if (CheckLuaVarCompat(LuaVar)) return LuaVar;
+    // All tests passed: id is a valid Lua identifier!
+    return true;
+}
 
-    // Add a leading underscore which will make the variable compatible in any case.
-    LuaVar="_"+LuaVar;
 
-    wxASSERT(CheckLuaVarCompat(LuaVar));
+wxString CheckLuaIdentifier(const wxString& id)
+{
+    // If id is valid, return it unchanged.
+    if (IsLuaIdentifier(id)) return id;
 
-    return LuaVar;
+    // If id is empty, just come up with some valid identifier.
+    if (id=="") return "id";
+
+    // Replace all non-alphanumeric characters with an underscore.
+    wxString NewId=id;
+
+    const wxRegEx NotAlphaNumRegEx("\\W", wxRE_ADVANCED);
+    wxASSERT(NotAlphaNumRegEx.IsValid());
+
+    NotAlphaNumRegEx.Replace(&NewId, "_");
+
+    if (IsLuaIdentifier(NewId)) return NewId;
+
+    // Still not good? Add a prefix.
+    NewId="id_"+NewId;
+
+    wxASSERT(IsLuaIdentifier(NewId));
+    return NewId;
 }

cafu/trunk/CaWE/LuaAux.hpp

@@ -39 +39 @@
 
 
-/// Checks if a string is a valid Lua variable name.
-/// @param Varname The variable name to be checked.
-/// @return Whether the string is valid or not.
-bool CheckLuaVarCompat(const wxString& Varname);
+/// Determines if the given string is a valid Lua identifier.
+///
+/// @param id   The identifier name to check.
+/// @return Whether \c id is a valid Lua identifier.
+bool IsLuaIdentifier(const wxString& id);
 
-/// Converts a string into a valid Lua variable name by replacing
-/// all special invalid characters with underscores and/or adding
-/// underscores to the begining of the name to make it valid.
-/// @param Varname The variable name to convert.
-/// @return The converted valid Lua variable name.
-wxString MakeLuaVarName(const wxString& Varname);
+
+/// Determines if the given string is a valid Lua identifier.
+/// If valid, \c id is returned unchanged.
+/// Otherwise, for the return value a variant of \c id is created that is valid.
+///
+/// @param id   The identifier name to check.
+/// @return A valid Lua identifier derived from \c id.
+wxString CheckLuaIdentifier(const wxString& id);
 
 #endif

cafu/trunk/CaWE/MapEntity.cpp

@@ -409 +409 @@
             for (unsigned long Count=1; true; Count++)
             {
-                const wxString UniqueValue=MakeLuaVarName(m_Class->GetName())+wxString::Format("_%03lu", Count);
+                const wxString UniqueValue=CheckLuaIdentifier(m_Class->GetName())+wxString::Format("_%03lu", Count);
                 unsigned long  EntNr;
 

cafu/trunk/CaWE/ModelEditor/ObserverPattern.hpp

@@ -81 +81 @@
         /// Notifies the observer that a skin has changed.
         /// @param Subject   The model document with the model in which the skin has changed.
-        /// @param AnimNr    The number of the skin that has changed.
+        /// @param SkinNr    The number of the skin that has changed.
         virtual void Notify_SkinChanged(SubjectT* Subject, unsigned int SkinNr) { }