Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better zones UI #95

Open
wants to merge 13 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions source/EngineInterface/AuxiliaryDataParserService.cpp
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#include <cstring>

#include "AuxiliaryDataParserService.h"

#include "GeneralSettings.h"
Expand Down Expand Up @@ -768,6 +770,16 @@ namespace
std::string base = "simulation parameters.spots." + std::to_string(index) + ".";
auto& spot = parameters.spots[index];
auto& defaultSpot = defaultParameters.spots[index];

std::string currentName(spot.name);
std::string defaultName(defaultSpot.name);
encodeDecodeProperty(tree, currentName, defaultName, base + "name", parserTask);

if (parserTask == ParserTask::Decode){
currentName = currentName.substr(0, SIM_PARAM_SPOT_NAME_LENGTH - 1); // Leave space for null byte
strncpy(spot.name, currentName.c_str(), SIM_PARAM_SPOT_NAME_LENGTH);
}

encodeDecodeProperty(tree, spot.color, defaultSpot.color, base + "color", parserTask);
encodeDecodeProperty(tree, spot.posX, defaultSpot.posX, base + "pos.x", parserTask);
encodeDecodeProperty(tree, spot.posY, defaultSpot.posY, base + "pos.y", parserTask);
Expand Down
7 changes: 5 additions & 2 deletions source/EngineInterface/SimulationParametersSpot.h
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#pragma once

#include <cstdint>

#include <cstring>
#include "SimulationParametersSpotActivatedValues.h"
#include "SimulationParametersSpotValues.h"

Expand Down Expand Up @@ -86,8 +86,11 @@ union SpotShapeData
RectangularSpot rectangularSpot;
};

static constexpr size_t SIM_PARAM_SPOT_NAME_LENGTH = 32;

struct SimulationParametersSpot
{
char name[SIM_PARAM_SPOT_NAME_LENGTH]{};
uint32_t color = 0;
float posX = 0;
float posY = 0;
Expand Down Expand Up @@ -139,7 +142,7 @@ struct SimulationParametersSpot
}
}

return color == other.color && posX == other.posX && posY == other.posY && velX == other.velX && velY == other.velY
return (strcmp(name, other.name) == 0) && color == other.color && posX == other.posX && posY == other.posY && velX == other.velX && velY == other.velY
&& fadeoutRadius == other.fadeoutRadius && values == other.values && activatedValues == other.activatedValues && shapeType == other.shapeType;
}
bool operator!=(SimulationParametersSpot const& other) const { return !operator==(other); }
Expand Down
81 changes: 67 additions & 14 deletions source/Gui/SimulationParametersWindow.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#include "SimulationParametersWindow.h"

#include <cstring>

#include <ImFileDialog.h>
#include <imgui.h>
#include <Fonts/IconsFontAwesome5.h>
Expand Down Expand Up @@ -72,6 +74,12 @@ _SimulationParametersWindow::_SimulationParametersWindow(
for (int i = 0; i < CellFunction_Count; ++i) {
_cellFunctionStrings.emplace_back(Const::CellFunctionToStringMap.at(i));
}

_serialTabID = 0;
for (int i = 0; i < MAX_SPOTS; ++i) {
_spotNameStrings.emplace_back("");
_spotTabIDs.emplace_back(std::to_string(0));
}
}

_SimulationParametersWindow::~_SimulationParametersWindow()
Expand Down Expand Up @@ -101,8 +109,12 @@ void _SimulationParametersWindow::processIntern()

SimulationParametersSpot _SimulationParametersWindow::createSpot(SimulationParameters const& simParameters, int index)
{
auto worldSize = _simController->getWorldSize();
SimulationParametersSpot spot;

strncpy(spot.name, std::string("Zone " + std::to_string(index)).c_str(), SIM_PARAM_SPOT_NAME_LENGTH);

auto worldSize = _simController->getWorldSize();

spot.posX = toFloat(worldSize.x / 2);
spot.posY = toFloat(worldSize.y / 2);

Expand Down Expand Up @@ -148,6 +160,8 @@ void _SimulationParametersWindow::processToolbar()
ImGui::SameLine();
if (AlienImGui::ToolbarButton(ICON_FA_COPY)) {
_copiedParameters = _simController->getSimulationParameters();
_copiedSpotNameStrings = _spotNameStrings;
_copiedSpotTabIDs = _spotTabIDs;
printOverlayMessage("Simulation parameters copied");
}
AlienImGui::Tooltip("Copy simulation parameters");
Expand All @@ -157,6 +171,8 @@ void _SimulationParametersWindow::processToolbar()
if (AlienImGui::ToolbarButton(ICON_FA_PASTE)) {
_simController->setSimulationParameters(*_copiedParameters);
_simController->setOriginalSimulationParameters(*_copiedParameters);
_spotNameStrings = _copiedSpotNameStrings;
_spotTabIDs = _copiedSpotTabIDs;
printOverlayMessage("Simulation parameters pasted");
}
ImGui::EndDisabled();
Expand All @@ -171,10 +187,29 @@ void _SimulationParametersWindow::processTabWidget(
SimulationParameters& origParameters)
{
auto currentSessionId = _simController->getSessionId();

bool sessionChanged = (!_sessionId.has_value() || (currentSessionId != *_sessionId));
_focusBaseTab = sessionChanged;

if (sessionChanged){
for (int i = 0; i < parameters.numSpots; ++i) {
// Legacy support: version <= 4.9 do not have names for zones
if (strcmp(parameters.spots[i].name, "") == 0){
_spotNameStrings[i] = std::string("Zone ") + std::to_string(i);
}else{
_spotNameStrings[i] = parameters.spots[i].name;
}
_spotTabIDs[i] = std::to_string(_serialTabID);
++_serialTabID;
}
}

_sessionId= currentSessionId;


if (ImGui::BeginChild("##", ImVec2(0, 0), false)) {

if (ImGui::BeginTabBar("##Parameters", ImGuiTabBarFlags_AutoSelectNewTabs | ImGuiTabBarFlags_FittingPolicyResizeDown)) {
if (ImGui::BeginTabBar("##Parameters", ImGuiTabBarFlags_AutoSelectNewTabs | ImGuiTabBarFlags_FittingPolicyResizeDown | ImGuiTabBarFlags_Reorderable)) {

//add spot
if (parameters.numSpots < MAX_SPOTS) {
Expand All @@ -184,47 +219,51 @@ void _SimulationParametersWindow::processTabWidget(
origParameters.spots[index] = createSpot(parameters, index);
++parameters.numSpots;
++origParameters.numSpots;

_spotNameStrings[index] = parameters.spots[index].name;
_spotTabIDs[index] = std::to_string(_serialTabID);
++_serialTabID;

_simController->setSimulationParameters(parameters);
_simController->setOriginalSimulationParameters(origParameters);
}
AlienImGui::Tooltip("Add parameter zone");
}

bool open = true;
if (ImGui::BeginTabItem("Base", &open, _focusBaseTab ? ImGuiTabItemFlags_SetSelected : ImGuiTabItemFlags_None)) {
if (ImGui::BeginTabItem("Base", nullptr, _focusBaseTab ? ImGuiTabItemFlags_SetSelected : ImGuiTabItemFlags_NoReorder)) {
processBase(parameters, origParameters);
ImGui::EndTabItem();
}

for (int tab = 0; tab < parameters.numSpots; ++tab) {
for (int tab = 0; tab < parameters.numSpots;) {
SimulationParametersSpot& spot = parameters.spots[tab];
SimulationParametersSpot const& origSpot = origParameters.spots[tab];
std::string name = "Zone " + std::to_string(tab + 1);
if (ImGui::BeginTabItem(name.c_str(), &open, ImGuiTabItemFlags_None)) {
processSpot(spot, origSpot, parameters);

bool open = true;
std::string label = std::string(_spotNameStrings[tab] + std::string("###") + _spotTabIDs[tab]);
if (ImGui::BeginTabItem(label.c_str(), &open, ImGuiTabItemFlags_None)) {
processSpot(tab,spot, origSpot, parameters);
ImGui::EndTabItem();
}

//delete spot
Copy link
Owner

Choose a reason for hiding this comment

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

Please restore this comment.

if (!open) {
for (int i = tab; i < parameters.numSpots - 1; ++i) {
_spotNameStrings[i] = _spotNameStrings[i + 1];
_spotTabIDs[i] = _spotTabIDs[i+1];
parameters.spots[i] = parameters.spots[i + 1];
origParameters.spots[i] = origParameters.spots[i + 1];
}
--parameters.numSpots;
--origParameters.numSpots;
_simController->setSimulationParameters(parameters);
_simController->setOriginalSimulationParameters(origParameters);
}else{
Copy link
Owner

Choose a reason for hiding this comment

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

I suppose there was a bug before with the counting of tab. Good that you have fixed it!

++tab;
}
}

ImGui::EndTabBar();
}
}
ImGui::EndChild();

_focusBaseTab = !_sessionId.has_value() || currentSessionId != *_sessionId;
_sessionId= currentSessionId;
}

void _SimulationParametersWindow::processBase(
Expand Down Expand Up @@ -1442,13 +1481,22 @@ void _SimulationParametersWindow::processBase(
}

void _SimulationParametersWindow::processSpot(
int tab,
SimulationParametersSpot& spot,
SimulationParametersSpot const& origSpot,
SimulationParameters const& parameters)
{
if (ImGui::BeginChild("##", ImVec2(0, 0), false, ImGuiWindowFlags_HorizontalScrollbar)) {
auto worldSize = _simController->getWorldSize();

if (AlienImGui::BeginTreeNode(AlienImGui::TreeNodeParameters().text("General"))) {
if(AlienImGui::InputText(AlienImGui::InputTextParameters().name("Name").textWidth(RightColumnWidth), _spotNameStrings[tab])){
_spotNameStrings[tab] = _spotNameStrings[tab].substr(0, SIM_PARAM_SPOT_NAME_LENGTH - 1); // Leave space for null byte
strncpy(spot.name, _spotNameStrings[tab].c_str(), SIM_PARAM_SPOT_NAME_LENGTH);
Copy link
Owner

Choose a reason for hiding this comment

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

Resharper tells me that strncpy is marked as deprecated because this function can be unsafe. One should use strncpy_s instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

strncpy_s, as well as the other _s variants, have an implementation in MSVC, but not in GCC. Thus, it is not portable and will not compile on the CI build, nor many standard linux distros. See 7421385. There are alternatives to the current method, such as using a library or using memcpy with strlen. I am agnostic to any of these options.

Copy link
Owner

Choose a reason for hiding this comment

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

Oh all right, I didn't know that because I use MSVC on my machine.

}
AlienImGui::EndTreeNode();
}

/**
* Colors and location
*/
Expand Down Expand Up @@ -2151,6 +2199,11 @@ void _SimulationParametersWindow::onOpenParameters()
if (!SerializerService::deserializeSimulationParametersFromFile(parameters, firstFilename.string())) {
MessageDialog::getInstance().information("Open simulation parameters", "The selected file could not be opened.");
} else {
for(int spot = 0; spot < parameters.numSpots; ++spot){
_spotNameStrings[spot] = std::string(parameters.spots[spot].name);
_spotTabIDs[spot] = std::to_string(_serialTabID);
++_serialTabID;
}
_simController->setSimulationParameters(parameters);
}
});
Expand Down
11 changes: 9 additions & 2 deletions source/Gui/SimulationParametersWindow.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class _SimulationParametersWindow : public _AlienWindow
void processToolbar();
void processTabWidget(SimulationParameters& parameters, SimulationParameters const& lastParameters, SimulationParameters& origParameters);
void processBase(SimulationParameters& parameters, SimulationParameters const& origParameters);
void processSpot(SimulationParametersSpot& spot, SimulationParametersSpot const& origSpot, SimulationParameters const& parameters);
void processSpot(int tab, SimulationParametersSpot& spot, SimulationParametersSpot const& origSpot, SimulationParameters const& parameters);
void processAddonList(SimulationParameters& parameters, SimulationParameters const& lastParameters, SimulationParameters const& origParameters);

void onOpenParameters();
Expand All @@ -41,7 +41,14 @@ class _SimulationParametersWindow : public _AlienWindow
std::optional<int> _sessionId;
bool _focusBaseTab = false;
std::vector<std::string> _cellFunctionStrings;


std::vector<std::string> _spotNameStrings;
Copy link
Owner

Choose a reason for hiding this comment

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

Is this member really necessary (same for _copiedSpotNameStrings)? Couldn't it be extracted from the name member in SimulationParameterSpot? I ask because I would like to avoid redundant data because then one always has to remember to keep it up to date in the code (e.g. after rearranging zones, pasting parameters, etc.).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will look into this. It was necessary at one point to deal with an imgui bug, but I question if this remains the case.

std::vector<std::string> _spotTabIDs;
std::vector<std::string> _copiedSpotNameStrings;
std::vector<std::string> _copiedSpotTabIDs;

int _serialTabID;
Copy link
Owner

Choose a reason for hiding this comment

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

I would prefer to initialize PODs at the point where they are declared. So here it should probably be = 0.


bool _featureListOpen = false;
float _featureListHeight = 200.0f;
};
Loading