5805: Refactor editor server settings and automapper config lists using `CScrollRegion` r=def- a=Robyt3

- Refactor editor automapper config list using `CScrollRegion`. Also fix some other minor issues with this list.
  - Fix the currently selected item not being highlighted after opening the dialog.
  - Remove static limit of 255 automapper rules, after which the editor previously caused out-of-bounds accesses, by using the address of the automapper config name as button ID.
  - Before:  ![automapper old](https://user-images.githubusercontent.com/23437060/188277300-bd634c13-fb66-4348-abf4-9481e8fcaa3a.png)
  - After: ![automapper new](https://user-images.githubusercontent.com/23437060/188277303-0cb94442-f35d-4c1e-bd81-7f56052cede6.png)
- Refactor editor server settings list using `CScrollRegion`.
  - Before:  ![server_settings old](https://user-images.githubusercontent.com/23437060/188277264-8a0d16b5-838c-4bd1-b94c-f11091c68ccf.png)
  - After: ![server_settings new](https://user-images.githubusercontent.com/23437060/188277269-1fabf5a5-9daf-4ffe-893d-1f1690ba597f.png)



## Checklist

- [X] Tested the change ingame
- [X] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test (especially base/) or added coverage to integration test
- [X] Considered possible null pointers and out of bounds array indexing
- [X] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)


Co-authored-by: Robert Müller <robytemueller@gmail.com>
This commit is contained in:
bors[bot] 2022-09-03 21:34:32 +00:00 committed by GitHub
commit ecce22322e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 34 additions and 98 deletions

View file

@ -5422,60 +5422,34 @@ void CEditor::RenderServerSettingsEditor(CUIRect View, bool ShowServerSettingsEd
CUIRect ListBox; CUIRect ListBox;
View.Margin(1.0f, &ListBox); View.Margin(1.0f, &ListBox);
float ListHeight = 17.0f * m_Map.m_vSettings.size(); const float ButtonHeight = 15.0f;
static float s_ScrollValue = 0; const float ButtonMargin = 2.0f;
float ScrollDifference = ListHeight - ListBox.h; static CScrollRegion s_ScrollRegion;
vec2 ScrollOffset(0.0f, 0.0f);
CScrollRegionParams ScrollParams;
ScrollParams.m_ScrollUnit = (ButtonHeight + ButtonMargin) * 5.0f;
s_ScrollRegion.Begin(&ListBox, &ScrollOffset, &ScrollParams);
ListBox.y += ScrollOffset.y;
if(ListHeight > ListBox.h) // Do we even need a scrollbar?
{
CUIRect Scroll;
ListBox.VSplitRight(20.0f, &ListBox, &Scroll);
s_ScrollValue = UI()->DoScrollbarV(&s_ScrollValue, &Scroll, s_ScrollValue);
if(UI()->MouseInside(&Scroll) || UI()->MouseInside(&ListBox))
{
int ScrollNum = (int)((ListHeight - ListBox.h) / 17.0f) + 1;
if(ScrollNum > 0)
{
if(Input()->KeyPress(KEY_MOUSE_WHEEL_UP))
s_ScrollValue = clamp(s_ScrollValue - 1.0f / ScrollNum, 0.0f, 1.0f);
if(Input()->KeyPress(KEY_MOUSE_WHEEL_DOWN))
s_ScrollValue = clamp(s_ScrollValue + 1.0f / ScrollNum, 0.0f, 1.0f);
}
}
}
float ListStartAt = ScrollDifference * s_ScrollValue;
if(ListStartAt < 0.0f)
ListStartAt = 0.0f;
float ListStopAt = ListHeight - ScrollDifference * (1 - s_ScrollValue);
float ListCur = 0;
UI()->ClipEnable(&ListBox);
for(size_t i = 0; i < m_Map.m_vSettings.size(); i++) for(size_t i = 0; i < m_Map.m_vSettings.size(); i++)
{
if(ListCur > ListStopAt)
break;
if(ListCur >= ListStartAt)
{ {
CUIRect Button; CUIRect Button;
ListBox.HSplitTop(15.0f, &Button, &ListBox); ListBox.HSplitTop(ButtonHeight, &Button, &ListBox);
ListBox.HSplitTop(2.0f, nullptr, &ListBox); ListBox.HSplitTop(ButtonMargin, nullptr, &ListBox);
Button.VSplitLeft(5.0f, nullptr, &Button); Button.VSplitLeft(5.0f, nullptr, &Button);
if(s_ScrollRegion.AddRect(Button))
{
if(DoButton_MenuItem(&m_Map.m_vSettings[i], m_Map.m_vSettings[i].m_aCommand, s_CommandSelectedIndex >= 0 && (size_t)s_CommandSelectedIndex == i, &Button, 0, nullptr)) if(DoButton_MenuItem(&m_Map.m_vSettings[i], m_Map.m_vSettings[i].m_aCommand, s_CommandSelectedIndex >= 0 && (size_t)s_CommandSelectedIndex == i, &Button, 0, nullptr))
{ {
s_CommandSelectedIndex = i; s_CommandSelectedIndex = i;
str_copy(m_aSettingsCommand, m_Map.m_vSettings[i].m_aCommand, sizeof(m_aSettingsCommand)); str_copy(m_aSettingsCommand, m_Map.m_vSettings[i].m_aCommand);
UI()->SetActiveItem(&m_CommandBox); UI()->SetActiveItem(&m_CommandBox);
} }
} }
ListCur += 17.0f;
} }
UI()->ClipDisable();
s_ScrollRegion.End();
} }
int CEditor::PopupMenuFile(CEditor *pEditor, CUIRect View, void *pContext) int CEditor::PopupMenuFile(CEditor *pEditor, CUIRect View, void *pContext)

View file

@ -1472,70 +1472,34 @@ static int s_AutoMapConfigCurrent = -100;
int CEditor::PopupSelectConfigAutoMap(CEditor *pEditor, CUIRect View, void *pContext) int CEditor::PopupSelectConfigAutoMap(CEditor *pEditor, CUIRect View, void *pContext)
{ {
CLayerTiles *pLayer = static_cast<CLayerTiles *>(pEditor->GetSelectedLayer(0)); CLayerTiles *pLayer = static_cast<CLayerTiles *>(pEditor->GetSelectedLayer(0));
CUIRect Button;
static int s_aAutoMapperConfigButtons[256];
CAutoMapper *pAutoMapper = &pEditor->m_Map.m_vpImages[pLayer->m_Image]->m_AutoMapper; CAutoMapper *pAutoMapper = &pEditor->m_Map.m_vpImages[pLayer->m_Image]->m_AutoMapper;
const float ButtonHeight = 12.0f; const float ButtonHeight = 12.0f;
const float ButtonMargin = 2.0f; const float ButtonMargin = 2.0f;
static float s_ScrollValue = 0; static CScrollRegion s_ScrollRegion;
vec2 ScrollOffset(0.0f, 0.0f);
CScrollRegionParams ScrollParams;
ScrollParams.m_ScrollbarWidth = 10.0f;
ScrollParams.m_ScrollbarMargin = 3.0f;
ScrollParams.m_ScrollUnit = (ButtonHeight + ButtonMargin) * 5;
s_ScrollRegion.Begin(&View, &ScrollOffset, &ScrollParams);
View.y += ScrollOffset.y;
// Add 1 more for the "None" option. for(int i = -1; i < pAutoMapper->ConfigNamesNum(); i++)
float ListHeight = (ButtonHeight + ButtonMargin) * (pAutoMapper->ConfigNamesNum() + 1);
float ScrollDifference = ListHeight - View.h;
// Disable scrollbar if not needed.
if(ListHeight > View.h)
{
CUIRect Scroll;
View.VSplitRight(20.0f, &View, &Scroll);
s_ScrollValue = pEditor->UI()->DoScrollbarV(&s_ScrollValue, &Scroll, s_ScrollValue);
if(pEditor->UI()->MouseInside(&View) || pEditor->UI()->MouseInside(&Scroll))
{
int ScrollNum = (int)((ListHeight / ButtonHeight) + 1);
if(ScrollNum > 0)
{
if(pEditor->Input()->KeyPress(KEY_MOUSE_WHEEL_UP))
s_ScrollValue = clamp(s_ScrollValue - 1.0f / ScrollNum, 0.0f, 1.0f);
if(pEditor->Input()->KeyPress(KEY_MOUSE_WHEEL_DOWN))
s_ScrollValue = clamp(s_ScrollValue + 1.0f / ScrollNum, 0.0f, 1.0f);
}
}
// Margin for scrollbar
View.VSplitRight(2.0f, &View, nullptr);
}
float ListStartAt = ScrollDifference * s_ScrollValue;
if(ListStartAt < 0.0f)
ListStartAt = 0.0f;
float ListStopAt = ListHeight - ScrollDifference * (1 - s_ScrollValue);
float ListCur = 0;
pEditor->UI()->ClipEnable(&View);
for(int i = 0; i < pAutoMapper->ConfigNamesNum() + 1; i++)
{
if(ListCur > ListStopAt)
break;
if(ListCur >= ListStartAt)
{ {
CUIRect Button;
View.HSplitTop(ButtonMargin, nullptr, &View); View.HSplitTop(ButtonMargin, nullptr, &View);
View.HSplitTop(ButtonHeight, &Button, &View); View.HSplitTop(ButtonHeight, &Button, &View);
if(pEditor->DoButton_MenuItem(&s_aAutoMapperConfigButtons[i], i == 0 ? "None" : pAutoMapper->GetConfigName(i - 1), i == s_AutoMapConfigCurrent, &Button, 0, nullptr)) if(s_ScrollRegion.AddRect(Button))
{ {
static int s_NoneButton = 0;
if(pEditor->DoButton_MenuItem(i == -1 ? (void *)&s_NoneButton : pAutoMapper->GetConfigName(i), i == -1 ? "None" : pAutoMapper->GetConfigName(i), i == s_AutoMapConfigCurrent, &Button, 0, nullptr))
s_AutoMapConfigSelected = i; s_AutoMapConfigSelected = i;
} }
} }
ListCur += ButtonHeight + ButtonMargin;
}
pEditor->UI()->ClipDisable(); s_ScrollRegion.End();
return 0; return 0;
} }
@ -1546,9 +1510,7 @@ void CEditor::PopupSelectConfigAutoMapInvoke(int Current, float x, float y)
s_AutoMapConfigSelected = -100; s_AutoMapConfigSelected = -100;
s_AutoMapConfigCurrent = Current; s_AutoMapConfigCurrent = Current;
CLayerTiles *pLayer = static_cast<CLayerTiles *>(GetSelectedLayer(0)); CLayerTiles *pLayer = static_cast<CLayerTiles *>(GetSelectedLayer(0));
int ItemCount = m_Map.m_vpImages[pLayer->m_Image]->m_AutoMapper.ConfigNamesNum(); const int ItemCount = minimum(m_Map.m_vpImages[pLayer->m_Image]->m_AutoMapper.ConfigNamesNum(), 10);
if(ItemCount > 10)
ItemCount = 10;
// Width for buttons is 120, 15 is the scrollbar width, 2 is the margin between both. // Width for buttons is 120, 15 is the scrollbar width, 2 is the margin between both.
UiInvokePopupMenu(&s_AutoMapConfigSelectID, 0, x, y, 120.0f + 15.0f + 2.0f, 26.0f + 14.0f * ItemCount, PopupSelectConfigAutoMap); UiInvokePopupMenu(&s_AutoMapConfigSelectID, 0, x, y, 120.0f + 15.0f + 2.0f, 26.0f + 14.0f * ItemCount, PopupSelectConfigAutoMap);
} }
@ -1560,7 +1522,7 @@ int CEditor::PopupSelectConfigAutoMapResult()
s_AutoMapConfigCurrent = s_AutoMapConfigSelected; s_AutoMapConfigCurrent = s_AutoMapConfigSelected;
s_AutoMapConfigSelected = -100; s_AutoMapConfigSelected = -100;
return s_AutoMapConfigCurrent - 1; return s_AutoMapConfigCurrent;
} }
// DDRace // DDRace