From 50ecfb178feb1f074c23a92d7019fedb007969cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=BCller?= Date: Tue, 17 Jan 2023 22:39:31 +0100 Subject: [PATCH 1/3] Add `CLayer::IsEntitiesLayer` to reduce duplicate code --- src/game/editor/editor.h | 4 ++++ src/game/editor/layer_tiles.cpp | 11 ++++++++--- src/game/editor/popups.cpp | 4 ++-- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/game/editor/editor.h b/src/game/editor/editor.h index dc8c02c63..cce7912f3 100644 --- a/src/game/editor/editor.h +++ b/src/game/editor/editor.h @@ -153,6 +153,8 @@ public: virtual void BrushFlipY() {} virtual void BrushRotate(float Amount) {} + virtual bool IsEntitiesLayer() const { return false; } + virtual void Render(bool Tileset = false) {} virtual int RenderProperties(CUIRect *pToolbox) { return 0; } @@ -608,6 +610,8 @@ public: void Snap(CUIRect *pRect); void Clamp(RECTi *pRect); + virtual bool IsEntitiesLayer() const override; + virtual bool IsEmpty(CLayerTiles *pLayer); void BrushSelecting(CUIRect Rect) override; int BrushGrab(CLayerGroup *pBrush, CUIRect Rect) override; diff --git a/src/game/editor/layer_tiles.cpp b/src/game/editor/layer_tiles.cpp index 313a76b55..c03a8496c 100644 --- a/src/game/editor/layer_tiles.cpp +++ b/src/game/editor/layer_tiles.cpp @@ -177,6 +177,11 @@ void CLayerTiles::Clamp(RECTi *pRect) pRect->w = 0; } +bool CLayerTiles::IsEntitiesLayer() const +{ + return m_pEditor->m_Map.m_pGameLayer == this || m_pEditor->m_Map.m_pTeleLayer == this || m_pEditor->m_Map.m_pSpeedupLayer == this || m_pEditor->m_Map.m_pFrontLayer == this || m_pEditor->m_Map.m_pSwitchLayer == this || m_pEditor->m_Map.m_pTuneLayer == this; +} + bool CLayerTiles::IsEmpty(CLayerTiles *pLayer) { for(int y = 0; y < pLayer->m_Height; y++) @@ -683,12 +688,12 @@ int CLayerTiles::RenderProperties(CUIRect *pToolBox) { CUIRect Button; - bool IsGameLayer = (m_pEditor->m_Map.m_pGameLayer == this || m_pEditor->m_Map.m_pTeleLayer == this || m_pEditor->m_Map.m_pSpeedupLayer == this || m_pEditor->m_Map.m_pFrontLayer == this || m_pEditor->m_Map.m_pSwitchLayer == this || m_pEditor->m_Map.m_pTuneLayer == this); + const bool EntitiesLayer = IsEntitiesLayer(); CLayerGroup *pGroup = m_pEditor->m_Map.m_vpGroups[m_pEditor->m_SelectedGroup]; // Game tiles can only be constructed if the layer is relative to the game layer - if(!IsGameLayer && !(pGroup->m_OffsetX % 32) && !(pGroup->m_OffsetY % 32) && pGroup->m_ParallaxX == 100 && pGroup->m_ParallaxY == 100) + if(!EntitiesLayer && !(pGroup->m_OffsetX % 32) && !(pGroup->m_OffsetY % 32) && pGroup->m_ParallaxX == 100 && pGroup->m_ParallaxY == 100) { pToolBox->HSplitBottom(12.0f, pToolBox, &Button); static int s_ColclButton = 0; @@ -847,7 +852,7 @@ int CLayerTiles::RenderProperties(CUIRect *pToolBox) {nullptr}, }; - if(IsGameLayer) // remove the image and color properties if this is a game layer + if(EntitiesLayer) // remove the image and color properties if this is a game layer { aProps[PROP_IMAGE].m_pName = nullptr; aProps[PROP_COLOR].m_pName = nullptr; diff --git a/src/game/editor/popups.cpp b/src/game/editor/popups.cpp index f60554f72..c78fd91eb 100644 --- a/src/game/editor/popups.cpp +++ b/src/game/editor/popups.cpp @@ -479,8 +479,8 @@ int CEditor::PopupLayer(CEditor *pEditor, CUIRect View, void *pContext) {nullptr}, }; - // if(pEditor->m_Map.m_pGameLayer == pEditor->GetSelectedLayer(0)) // don't use Group and Detail from the selection if this is the game layer - if(pEditor->m_Map.m_pGameLayer == pEditor->GetSelectedLayer(0) || pEditor->m_Map.m_pTeleLayer == pEditor->GetSelectedLayer(0) || pEditor->m_Map.m_pSpeedupLayer == pEditor->GetSelectedLayer(0) || pEditor->m_Map.m_pFrontLayer == pEditor->GetSelectedLayer(0) || pEditor->m_Map.m_pSwitchLayer == pEditor->GetSelectedLayer(0) || pEditor->m_Map.m_pTuneLayer == pEditor->GetSelectedLayer(0)) // don't use Group and Detail from the selection if this is the game layer + // don't use Group and Detail from the selection if this is an entities layer + if(pEditor->GetSelectedLayer(0)->IsEntitiesLayer()) { aProps[0].m_Type = PROPTYPE_NULL; aProps[2].m_Type = PROPTYPE_NULL; From c7819841f8f70472d09f779f377fcb6dc2c27c0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=BCller?= Date: Sat, 21 Jan 2023 14:02:15 +0100 Subject: [PATCH 2/3] Remove dead code, update documentation for `LAYERTYPE_GAME` This layer type was never used when saving maps. Game layers were always identified by their flags. In any case the check in the popup handler is unnecessary, as the group property cannot be changed for game layers, so this code is never used. --- src/game/editor/popups.cpp | 2 +- src/game/mapitems.h | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/game/editor/popups.cpp b/src/game/editor/popups.cpp index c78fd91eb..0aa6b0f1d 100644 --- a/src/game/editor/popups.cpp +++ b/src/game/editor/popups.cpp @@ -494,7 +494,7 @@ int CEditor::PopupLayer(CEditor *pEditor, CUIRect View, void *pContext) if(Prop == PROP_ORDER) pEditor->SelectLayer(pCurrentGroup->SwapLayers(pEditor->m_vSelectedLayers[0], NewVal)); - else if(Prop == PROP_GROUP && pCurrentLayer && pCurrentLayer->m_Type != LAYERTYPE_GAME) + else if(Prop == PROP_GROUP && pCurrentLayer) { if(NewVal >= 0 && (size_t)NewVal < pEditor->m_Map.m_vpGroups.size()) { diff --git a/src/game/mapitems.h b/src/game/mapitems.h index 27348cc14..f462b122e 100644 --- a/src/game/mapitems.h +++ b/src/game/mapitems.h @@ -8,9 +8,8 @@ // layer types enum { - // TODO(Shereef Marzouk): fix this for vanilla, make use of LAYERTYPE_GAME instead of using m_game variable in the editor. LAYERTYPE_INVALID = 0, - LAYERTYPE_GAME, + LAYERTYPE_GAME, // unused LAYERTYPE_TILES, LAYERTYPE_QUADS, LAYERTYPE_FRONT, From 29b61fc438299c0dd2dfc5e3c873f80e32dcb3b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20M=C3=BCller?= Date: Sun, 22 Jan 2023 11:58:09 +0100 Subject: [PATCH 3/3] Refactor `CEditor:::PopupLayer`, remove `CEditor::IsSpecialLayer` - Use `pCurrentLayer` instead of `pEditor->GetSelectedLayer(0)` everywhere. - Remove unnecessary null-checks of `pCurrentLayer`. - Use `CLayer::IsEntitiesLayer` instead of `CEditor::IsSpecialLayer` and remove the latter function. - Restructure UI layout code and variables. --- src/game/editor/editor.cpp | 5 -- src/game/editor/editor.h | 1 - src/game/editor/popups.cpp | 93 +++++++++++++++++++------------------- 3 files changed, 47 insertions(+), 52 deletions(-) diff --git a/src/game/editor/editor.cpp b/src/game/editor/editor.cpp index 28d16cd61..9445b5556 100644 --- a/src/game/editor/editor.cpp +++ b/src/game/editor/editor.cpp @@ -782,11 +782,6 @@ int CEditor::FindSelectedQuadIndex(int Index) const return -1; } -bool CEditor::IsSpecialLayer(const CLayer *pLayer) const -{ - return m_Map.m_pGameLayer == pLayer || m_Map.m_pTeleLayer == pLayer || m_Map.m_pSpeedupLayer == pLayer || m_Map.m_pFrontLayer == pLayer || m_Map.m_pSwitchLayer == pLayer || m_Map.m_pTuneLayer == pLayer; -} - void CEditor::CallbackOpenMap(const char *pFileName, int StorageType, void *pUser) { CEditor *pEditor = (CEditor *)pUser; diff --git a/src/game/editor/editor.h b/src/game/editor/editor.h index cce7912f3..fda17f45c 100644 --- a/src/game/editor/editor.h +++ b/src/game/editor/editor.h @@ -892,7 +892,6 @@ public: void DeleteSelectedQuads(); bool IsQuadSelected(int Index) const; int FindSelectedQuadIndex(int Index) const; - bool IsSpecialLayer(const CLayer *pLayer) const; float ScaleFontSize(char *pText, int TextSize, float FontSize, int Width); int DoProperties(CUIRect *pToolbox, CProperty *pProps, int *pIDs, int *pNewVal, ColorRGBA Color = ColorRGBA(1, 1, 1, 0.5f)); diff --git a/src/game/editor/popups.cpp b/src/game/editor/popups.cpp index 0aa6b0f1d..aa827db38 100644 --- a/src/game/editor/popups.cpp +++ b/src/game/editor/popups.cpp @@ -403,66 +403,69 @@ int CEditor::PopupLayer(CEditor *pEditor, CUIRect View, void *pContext) { CLayerPopupContext *pPopup = (CLayerPopupContext *)pContext; - // remove layer button - CUIRect Button; - View.HSplitBottom(12.0f, &View, &Button); - static int s_DeleteButton = 0; + CLayerGroup *pCurrentGroup = pEditor->m_Map.m_vpGroups[pEditor->m_SelectedGroup]; + CLayer *pCurrentLayer = pEditor->GetSelectedLayer(0); if(pPopup->m_vpLayers.size() > 1) { return CLayerTiles::RenderCommonProperties(pPopup->m_CommonPropState, pEditor, &View, pPopup->m_vpLayers); } - // duplicate layer button - CUIRect DupButton; - static int s_DuplicationButton = 0; - View.HSplitBottom(4.0f, &View, nullptr); - View.HSplitBottom(12.0f, &View, &DupButton); + const bool EntitiesLayer = pCurrentLayer->IsEntitiesLayer(); - if(!pEditor->IsSpecialLayer(pEditor->GetSelectedLayer(0))) + // delete button + if(pEditor->m_Map.m_pGameLayer != pCurrentLayer) // entities layers except the game layer can be deleted { - if(pEditor->DoButton_Editor(&s_DuplicationButton, "Duplicate layer", 0, &DupButton, 0, "Duplicates the layer")) + CUIRect DeleteButton; + View.HSplitBottom(12.0f, &View, &DeleteButton); + static int s_DeleteButton = 0; + if(pEditor->DoButton_Editor(&s_DeleteButton, "Delete layer", 0, &DeleteButton, 0, "Deletes the layer")) + { + if(pCurrentLayer == pEditor->m_Map.m_pFrontLayer) + pEditor->m_Map.m_pFrontLayer = nullptr; + if(pCurrentLayer == pEditor->m_Map.m_pTeleLayer) + pEditor->m_Map.m_pTeleLayer = nullptr; + if(pCurrentLayer == pEditor->m_Map.m_pSpeedupLayer) + pEditor->m_Map.m_pSpeedupLayer = nullptr; + if(pCurrentLayer == pEditor->m_Map.m_pSwitchLayer) + pEditor->m_Map.m_pSwitchLayer = nullptr; + if(pCurrentLayer == pEditor->m_Map.m_pTuneLayer) + pEditor->m_Map.m_pTuneLayer = nullptr; + pEditor->m_Map.m_vpGroups[pEditor->m_SelectedGroup]->DeleteLayer(pEditor->m_vSelectedLayers[0]); + return 1; + } + } + + // duplicate button + if(!EntitiesLayer) // entities layers cannot be duplicated + { + CUIRect DuplicateButton; + View.HSplitBottom(4.0f, &View, nullptr); + View.HSplitBottom(12.0f, &View, &DuplicateButton); + static int s_DuplicationButton = 0; + if(pEditor->DoButton_Editor(&s_DuplicationButton, "Duplicate layer", 0, &DuplicateButton, 0, "Duplicates the layer")) { pEditor->m_Map.m_vpGroups[pEditor->m_SelectedGroup]->DuplicateLayer(pEditor->m_vSelectedLayers[0]); return 1; } } - // don't allow deletion of game layer - if(pEditor->m_Map.m_pGameLayer != pEditor->GetSelectedLayer(0) && - pEditor->DoButton_Editor(&s_DeleteButton, "Delete layer", 0, &Button, 0, "Deletes the layer")) - { - if(pEditor->GetSelectedLayer(0) == pEditor->m_Map.m_pFrontLayer) - pEditor->m_Map.m_pFrontLayer = nullptr; - if(pEditor->GetSelectedLayer(0) == pEditor->m_Map.m_pTeleLayer) - pEditor->m_Map.m_pTeleLayer = nullptr; - if(pEditor->GetSelectedLayer(0) == pEditor->m_Map.m_pSpeedupLayer) - pEditor->m_Map.m_pSpeedupLayer = nullptr; - if(pEditor->GetSelectedLayer(0) == pEditor->m_Map.m_pSwitchLayer) - pEditor->m_Map.m_pSwitchLayer = nullptr; - if(pEditor->GetSelectedLayer(0) == pEditor->m_Map.m_pTuneLayer) - pEditor->m_Map.m_pTuneLayer = nullptr; - pEditor->m_Map.m_vpGroups[pEditor->m_SelectedGroup]->DeleteLayer(pEditor->m_vSelectedLayers[0]); - return 1; - } - // layer name - // if(pEditor->m_Map.m_pGameLayer != pEditor->GetSelectedLayer(0)) - if(pEditor->m_Map.m_pGameLayer != pEditor->GetSelectedLayer(0) && pEditor->m_Map.m_pTeleLayer != pEditor->GetSelectedLayer(0) && pEditor->m_Map.m_pSpeedupLayer != pEditor->GetSelectedLayer(0) && pEditor->m_Map.m_pFrontLayer != pEditor->GetSelectedLayer(0) && pEditor->m_Map.m_pSwitchLayer != pEditor->GetSelectedLayer(0) && pEditor->m_Map.m_pTuneLayer != pEditor->GetSelectedLayer(0)) + if(!EntitiesLayer) // name cannot be changed for entities layers { - View.HSplitBottom(5.0f, &View, &Button); - View.HSplitBottom(12.0f, &View, &Button); - pEditor->UI()->DoLabel(&Button, "Name:", 10.0f, TEXTALIGN_LEFT); - Button.VSplitLeft(40.0f, nullptr, &Button); + CUIRect Label, EditBox; + View.HSplitBottom(5.0f, &View, nullptr); + View.HSplitBottom(12.0f, &View, &Label); + Label.VSplitLeft(40.0f, &Label, &EditBox); + pEditor->UI()->DoLabel(&Label, "Name:", 10.0f, TEXTALIGN_LEFT); static float s_Name = 0; - if(pEditor->DoEditBox(&s_Name, &Button, pEditor->GetSelectedLayer(0)->m_aName, sizeof(pEditor->GetSelectedLayer(0)->m_aName), 10.0f, &s_Name)) + if(pEditor->DoEditBox(&s_Name, &EditBox, pCurrentLayer->m_aName, sizeof(pCurrentLayer->m_aName), 10.0f, &s_Name)) pEditor->m_Map.m_Modified = true; } - View.HSplitBottom(10.0f, &View, nullptr); - - CLayerGroup *pCurrentGroup = pEditor->m_Map.m_vpGroups[pEditor->m_SelectedGroup]; - CLayer *pCurrentLayer = pEditor->GetSelectedLayer(0); + // spacing if any button was rendered + if(!EntitiesLayer || pEditor->m_Map.m_pGameLayer != pCurrentLayer) + View.HSplitBottom(10.0f, &View, nullptr); enum { @@ -475,12 +478,12 @@ int CEditor::PopupLayer(CEditor *pEditor, CUIRect View, void *pContext) CProperty aProps[] = { {"Group", pEditor->m_SelectedGroup, PROPTYPE_INT_STEP, 0, (int)pEditor->m_Map.m_vpGroups.size() - 1}, {"Order", pEditor->m_vSelectedLayers[0], PROPTYPE_INT_STEP, 0, (int)pCurrentGroup->m_vpLayers.size() - 1}, - {"Detail", pCurrentLayer && pCurrentLayer->m_Flags & LAYERFLAG_DETAIL, PROPTYPE_BOOL, 0, 1}, + {"Detail", pCurrentLayer->m_Flags & LAYERFLAG_DETAIL, PROPTYPE_BOOL, 0, 1}, {nullptr}, }; // don't use Group and Detail from the selection if this is an entities layer - if(pEditor->GetSelectedLayer(0)->IsEntitiesLayer()) + if(EntitiesLayer) { aProps[0].m_Type = PROPTYPE_NULL; aProps[2].m_Type = PROPTYPE_NULL; @@ -494,7 +497,7 @@ int CEditor::PopupLayer(CEditor *pEditor, CUIRect View, void *pContext) if(Prop == PROP_ORDER) pEditor->SelectLayer(pCurrentGroup->SwapLayers(pEditor->m_vSelectedLayers[0], NewVal)); - else if(Prop == PROP_GROUP && pCurrentLayer) + else if(Prop == PROP_GROUP) { if(NewVal >= 0 && (size_t)NewVal < pEditor->m_Map.m_vpGroups.size()) { @@ -506,15 +509,13 @@ int CEditor::PopupLayer(CEditor *pEditor, CUIRect View, void *pContext) pEditor->SelectLayer(pEditor->m_Map.m_vpGroups[NewVal]->m_vpLayers.size() - 1); } } - else if(Prop == PROP_HQ && pCurrentLayer) + else if(Prop == PROP_HQ) { pCurrentLayer->m_Flags &= ~LAYERFLAG_DETAIL; if(NewVal) pCurrentLayer->m_Flags |= LAYERFLAG_DETAIL; } - if(!pCurrentLayer) - return true; return pCurrentLayer->RenderProperties(&View); }