HomeWildfire Games

Use victory conditions json's in Atlas
AuditedrP23847

Description

Use victory conditions json's in Atlas

Reviewed by: @Angen

Differential Revision: https://code.wildfiregames.com/D2393

Event Timeline

Stan added a comment.Jul 18 2020, 7:44 PM

Clang

In file included from ../../../source/tools/atlas/AtlasUI/ScenarioEditor/Sections/Map/Map.cpp:23:
In file included from ../../../source/tools/atlas/AtlasObject/JSONSpiritInclude.h:32:
In file included from ../../../source/third_party/jsonspirit/json_spirit_writer_template.h:13:
../../../source/third_party/jsonspirit/json_spirit_value.h:586:24: warning: 'static' function 'value_type_to_string' declared in header file should be declared 'static inline' [-Wunneeded-internal-declaration]
    static std::string value_type_to_string( const Value_type vtype )
                       ^
In file included from ../../../source/tools/atlas/AtlasUI/ScenarioEditor/Sections/Map/Map.cpp:23:
In file included from ../../../source/tools/atlas/AtlasObject/JSONSpiritInclude.h:32:
../../../source/third_party/jsonspirit/json_spirit_writer_template.h:37:50: warning: unused typedef 'Char_type' [-Wunused-local-typedef]

        typedef typename String_type::value_type Char_type;

Windows:

8>e:\jenkins\workspace\vs2015-differential\source\third_party\jsonspirit\json_spirit_value.h(586): warning C4505: 'json_spirit::value_type_to_string': unreferenced local function has been removed (compiling source file ..\..\..\source\tools\atlas\AtlasUI\ScenarioEditor\Sections\Map\Map.cpp) [E:\Jenkins\workspace\vs2015-differential\build\workspaces\vc2015\AtlasUI.vcxproj]

GCC

vladislavbelov raised a concern with this commit.Jul 21 2020, 3:22 AM
vladislavbelov added a subscriber: vladislavbelov.

The change has broken map settings editing.

/ps/trunk/source/tools/atlas/AtlasUI/ScenarioEditor/Sections/Map/Map.cpp
107

It's wrong to call OnVictoryConditionChanged for each object on the panel. You have to call it only in case evt.GetId() is in m_VictoryConditions. Because multiple elements can send OnEdit:

BEGIN_EVENT_TABLE(MapSettingsControl, wxPanel)
	EVT_TEXT(ID_MapName, MapSettingsControl::OnEdit)
	EVT_TEXT(ID_MapDescription, MapSettingsControl::OnEdit)
	EVT_TEXT(ID_MapPreview, MapSettingsControl::OnEdit)
	EVT_CHECKBOX(wxID_ANY, MapSettingsControl::OnEdit)
	EVT_CHOICE(wxID_ANY, MapSettingsControl::OnEdit)
END_EVENT_TABLE();
316

The atlas crashes here, because you're trying to cast wxTextCtrl to wxCheckBox.

This commit now has outstanding concerns.Jul 21 2020, 3:22 AM
Stan requested verification of this commit.Jul 24 2020, 8:53 PM
Stan marked an inline comment as done.
This commit now requires verification by auditors.Jul 24 2020, 8:53 PM
All concerns with this commit have now been addressed.Jul 25 2020, 1:58 AM
Silier raised a concern with this commit.Aug 26 2020, 9:49 PM
Silier added inline comments.
/ps/trunk/source/tools/atlas/AtlasUI/ScenarioEditor/Sections/Map/Map.cpp
306

Atlas is saving conditions without '_' so it is broken, meh how did I miss it.

This commit now has outstanding concerns.Aug 26 2020, 9:49 PM
Silier accepted this commit.Sep 20 2020, 11:41 AM
All concerns with this commit have now been addressed.Sep 20 2020, 11:41 AM