Page MenuHomeWildfire Games

Atlas variation submenu
Needs ReviewPublic

Authored by trompetin17 on Jul 5 2019, 6:32 PM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Trac Tickets
#2472
Summary

Enable Variation panel, map writer, map reader and CUnit actorSelections

Test Plan

Apply patch:
create new map, add entity, play with variation, save and reload the map
Load a map, play tiwh variation, save and reload the map

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 8371
Build 13672: Vulcan BuildJenkins
Build 13671: arc lint + arc unit

Event Timeline

trompetin17 created this revision.Jul 5 2019, 6:32 PM
trompetin17 edited the test plan for this revision. (Show Details)Jul 5 2019, 6:33 PM
Vulcan added a comment.Jul 5 2019, 6:34 PM

Build failure - The Moirai have given mortals hearts that can endure.

Linter detected issues:
Executing section Source...

source/graphics/MapReader.cpp
|   1| /*·Copyright·(C)·2017·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2017"

source/tools/atlas/GameInterface/Messages.h
| 211| »   »   );
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Invalid number of character '{' when these macros are defined: 'MESSAGESSETUP_NOTFIRST'.

source/tools/atlas/GameInterface/Handlers/ObjectHandlers.cpp
| 211| »   {
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Invalid number of character '{' when these macros are defined: 'MESSAGESSETUP_NOTFIRST'.
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/differential/1901/display/redirect

trompetin17 updated this revision to Diff 8726.Jul 5 2019, 6:40 PM

Fixing std::string initialize and check empty

Angen added a subscriber: Angen.Jul 5 2019, 6:40 PM

please remove not needed braces for one command after if condition, will test once it builds

source/graphics/MapWriter.cpp
394–395

please remove { }

420

remove { }

source/tools/atlas/GameInterface/Handlers/ObjectHandlers.cpp
336

remove { }

375

remove { }

Vulcan added a comment.Jul 5 2019, 6:41 PM

Build failure - The Moirai have given mortals hearts that can endure.

Linter detected issues:
Executing section Source...

source/graphics/MapReader.cpp
|   1| /*·Copyright·(C)·2017·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2017"

source/tools/atlas/GameInterface/Messages.h
| 211| »   »   );
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Invalid number of character '{' when these macros are defined: 'MESSAGESSETUP_NOTFIRST'.

source/tools/atlas/GameInterface/Handlers/ObjectHandlers.cpp
| 211| »   {
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Invalid number of character '{' when these macros are defined: 'MESSAGESSETUP_NOTFIRST'.
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/differential/1902/display/redirect

trompetin17 updated this revision to Diff 8727.Jul 5 2019, 6:44 PM

remove not needed braces for one command after if condition,

trompetin17 marked 3 inline comments as done.Jul 5 2019, 6:45 PM
Vulcan added a comment.Jul 5 2019, 6:45 PM

Build failure - The Moirai have given mortals hearts that can endure.

Linter detected issues:
Executing section Source...

source/graphics/MapReader.cpp
|   1| /*·Copyright·(C)·2017·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2017"

source/tools/atlas/GameInterface/Messages.h
| 211| »   »   );
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Invalid number of character '{' when these macros are defined: 'MESSAGESSETUP_NOTFIRST'.

source/tools/atlas/GameInterface/Handlers/ObjectHandlers.cpp
| 211| »   {
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Invalid number of character '{' when these macros are defined: 'MESSAGESSETUP_NOTFIRST'.
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/differential/1903/display/redirect

trompetin17 updated this revision to Diff 8732.Jul 5 2019, 6:51 PM

Update Copyright

Angen added inline comments.Jul 5 2019, 6:54 PM
source/graphics/MapReader.cpp
1018

I believe same here about { }

1072

this { } can go away

Vulcan added a comment.Jul 5 2019, 6:55 PM

Build failure - The Moirai have given mortals hearts that can endure.

Linter detected issues:
Executing section Source...

source/tools/atlas/GameInterface/Handlers/ObjectHandlers.cpp
| 211| »   {
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Invalid number of character '{' when these macros are defined: 'MESSAGESSETUP_NOTFIRST'.
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/differential/1905/display/redirect

Angen added a comment.Jul 5 2019, 6:57 PM

It builds for me on windows but not with jenkins, he has linux

source/graphics/MapReader.cpp
31

jenkins does not like this include

../../../source/graphics/MapReader.cpp:31:27: fatal error: graphics/unit.h: No such file or directory

 #include "graphics/unit.h"
trompetin17 updated this revision to Diff 8733.Jul 5 2019, 7:24 PM

Fixing unit.h to Unit.h, remove some braces

trompetin17 marked 4 inline comments as done.Jul 5 2019, 7:25 PM
Vulcan added a comment.Jul 5 2019, 7:26 PM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/tools/atlas/GameInterface/Handlers/ObjectHandlers.cpp
| 211| »   {
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Invalid number of character '{' when these macros are defined: 'MESSAGESSETUP_NOTFIRST'.
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/differential/1907/display/redirect


Evidence about variation!

trompetin17 updated this revision to Diff 8745.Jul 6 2019, 8:58 AM

Change atribute variations to node element

trompetin17 updated this revision to Diff 8746.Jul 6 2019, 9:01 AM

Remove backspace

Vulcan added a comment.Jul 6 2019, 9:32 AM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/tools/atlas/GameInterface/Handlers/ObjectHandlers.cpp
| 211| »   {
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Invalid number of character '{' when these macros are defined: 'MESSAGESSETUP_NOTFIRST'.
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/differential/1917/display/redirect

Vulcan added a comment.Jul 6 2019, 9:35 AM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/tools/atlas/GameInterface/Handlers/ObjectHandlers.cpp
| 211| »   {
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Invalid number of character '{' when these macros are defined: 'MESSAGESSETUP_NOTFIRST'.
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/differential/1918/display/redirect

elexis added a subscriber: elexis.Jul 8 2019, 5:50 PM

Thanks for having taken into account my IRC review remarks in the past days about the "|" concatenation in the attribute setting vs. individual XML nodes.

I consulted the findings from rP22335 again to see if UTF8 handling is proper, seems so.

When opening the Arcadia map:

XeroXMB.cpp(257): Assertion failed: "id < m_Size && "Element ID out of range""
Assertion failed: "id < m_Size && "Element ID out of range""
Location: XeroXMB.cpp:257 (operator[])
elexis added inline comments.Jul 14 2019, 9:59 PM
source/graphics/MapReader.cpp
1018

I suspect it complains about childActors[0] being out of bounds

trompetin17 updated this revision to Diff 8906.Jul 15 2019, 5:42 AM

Add validation on childActors, it must have one child

trompetin17 marked an inline comment as done.Jul 15 2019, 5:43 AM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/tools/atlas/GameInterface/Handlers/ObjectHandlers.cpp
|1055| »   »   »   oldObjects.push_back(obj);
|    | [MAJOR] CPPCheckBear (uninitStructMember):
|    | Uninitialized struct member: obj.owner

source/tools/atlas/GameInterface/Handlers/ObjectHandlers.cpp
| 211| »   {
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Unmatched '}'. Configuration: 'MESSAGESSETUP_NOTFIRST'.
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/81/display/redirect

elexis added inline comments.Jul 15 2019, 12:53 PM
source/graphics/MapReader.cpp
1014

it warns about > 1, perhaps it should also warn about 0?

Perhaps it would be good to display the names of the variations. For example if one selects a cavalry unit, there are a range of colors settings, but one can't deduce which color refers to which property of the model.
(It's a bit unexpected that one can chose whether it's carrying meat or running, but maybe that's technically impossible to check)
Perhaps also more horizontal space for the panel, it looks a bit squeezed.

Very nice feature!

trompetin17 added inline comments.Jul 15 2019, 3:35 PM
source/graphics/MapReader.cpp
1014

could be, but if we have 0 children that means they didn't choose any variant and this could be acceptable