Page MenuHomeWildfire Games

Also store turret position in map file.
ClosedPublic

Authored by Freagarach on Feb 3 2020, 8:11 PM.

Details

Reviewers
wraitii
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP24161: Store turret positions in map files.
Summary

This is a follow-up from D2597 and saves the turret position in the map file.
This means one can specify the turret points of entities to occupy on init explicitly instead of relying on the order at which the entities are added.

Test Plan

Verify that:

  • When loading a map with a turret the turret is positioned at the correct location.
  • Loading e.g. the Silica Nomad map - which has no visibly garrioned entities - still works fine.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Freagarach created this revision.Feb 3 2020, 8:11 PM
Freagarach created this object with visibility "No One".
Owners added subscribers: Restricted Owners Package, Restricted Owners Package, Restricted Owners Package.Feb 3 2020, 8:11 PM
Freagarach retitled this revision from Also store turret position in map file. to [WIP] - Also store turret position in map file..
Freagarach changed the visibility from "No One" to "Custom Policy".
Freagarach retitled this revision from [WIP] - Also store turret position in map file. to Also store turret position in map file..
Freagarach edited the summary of this revision. (Show Details)
Freagarach edited the test plan for this revision. (Show Details)
Freagarach added a reviewer: Restricted Owners Package.
Freagarach changed the visibility from "Custom Policy" to "Public (No Login Required)".

Working version :)

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

Linter detected issues:
Executing section Source...

source/simulation2/components/ICmpGarrisonHolder.h
|  25| class·ICmpGarrisonHolder·:·public·IComponent
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classICmpGarrisonHolder:' is invalid C code. Use --std or --language to configure the language.
Executing section JS...
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/GarrisonHolder.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/GarrisonHolder.js
| 639| 639| 	}
| 640| 640| 	else if (this.initGarrison.has(msg.entity))
| 641| 641| 	{
| 642|    |-		this.initGarrison.set(msg.newentity, this.initGarrison.get(msg.entity))
|    | 642|+		this.initGarrison.set(msg.newentity, this.initGarrison.get(msg.entity));
| 643| 643| 		this.initGarrison.delete(msg.entity);
| 644| 644| 	}
| 645| 645| };

binaries/data/mods/public/simulation/components/GarrisonHolder.js
| 642| »   »   this.initGarrison.set(msg.newentity,·this.initGarrison.get(msg.entity))
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.
Executing section cli...

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

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

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/517/display/redirect

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

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/527/display/redirect

bb added a subscriber: bb.Jun 1 2020, 6:19 PM

Did you also do a rejoin test? We move some serialized data around, and remove a clone somewhere, so that raises a concern (not that I see it trivially broken though)

binaries/data/mods/public/simulation/components/GarrisonHolder.js
113 ↗(On Diff #11601)

lowercase s afaik

117–118 ↗(On Diff #11601)

Why do we need this check?

119 ↗(On Diff #11601)

in case we can't find a vgp with the wanted condition, we will error out, no?

Freagarach planned changes to this revision.Jun 4 2020, 8:26 AM

Comments and perhaps wait for D2367?

Freagarach updated this revision to Diff 12870.Jul 23 2020, 9:15 AM
Freagarach edited the summary of this revision. (Show Details)

Rebased.

Note that linking pyrogenesis will fail due to the script call.

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

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/2229/display/redirect

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

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1130/display/redirect

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

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

Freagarach added inline comments.Jul 23 2020, 12:43 PM
source/simulation2/components/ICmpTurretHolder.cpp
35 ↗(On Diff #12870)

I could cirvumvent by fetching a list of the turret IDs and then querying their position.

Freagarach planned changes to this revision.Jul 23 2020, 1:56 PM

Nicest would be to implement JS <-> C++ map conversion. (http://irclogs.wildfiregames.com/2020-07/2020-07-23-QuakeNet-%230ad-dev.log)

wraitii updated this revision to Diff 13107.Aug 6 2020, 3:42 PM
wraitii added a subscriber: wraitii.

Implement unordered_map conversion.

I can re-use it for D2814 and it seems useful to have a way to convert dictionaries as well as lists.

I think the code still wont' work 'cause there are likely other issues but you are now unstuck.

wraitii requested changes to this revision.Aug 6 2020, 3:44 PM

@Freagarach all yours

binaries/data/mods/public/simulation/components/TurretHolder.js
268 ↗(On Diff #13107)

Probably not the right variable.

source/graphics/MapReader.cpp
1031 ↗(On Diff #13107)

this you need a SetInitEntities call here?

This revision now requires changes to proceed.Aug 6 2020, 3:44 PM
Vulcan added a comment.Aug 6 2020, 3:49 PM

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

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

Freagarach added inline comments.Aug 6 2020, 7:56 PM
source/graphics/MapReader.cpp
1075 ↗(On Diff #13107)

Nah, I need to set init turrets here ^^

Angen added a subscriber: Angen.Aug 6 2020, 8:46 PM
Angen added inline comments.
binaries/data/mods/public/simulation/components/TurretHolder.js
215 ↗(On Diff #13107)

should be warning if you continue to execute function
error if you are aborting

vladislavbelov added inline comments.
binaries/data/mods/public/simulation/components/TurretHolder.js
194 ↗(On Diff #13107)

Why map? Especially if you use this map only in C++. I suggest to avoid non-trivial containers in JS conversions.

Freagarach added inline comments.Aug 7 2020, 7:42 AM
binaries/data/mods/public/simulation/components/TurretHolder.js
194 ↗(On Diff #13107)

Because I need to correlate the name of the turret point with the entityID occupying it.
I'd say I might also use a plain Object, with the turret names as keys and the entityID as value, but perhaps I am thinking too difficult? Could you give some insight please?

Freagarach updated this revision to Diff 13126.Aug 7 2020, 9:10 AM
Freagarach marked 3 inline comments as done.
  • Back to a vector of pairs.
  • Fully working version, though perhaps with some ugliness.
Vulcan added a comment.Aug 7 2020, 9:56 AM

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

Linter detected issues:
Executing section Source...
Executing section JS...
**** ESLintBear (semi) [Section <empty> | Severity NORMAL] ****
!    ! Missing semicolon.
[----] /zpool0/trunk/binaries/data/mods/public/simulation/components/TurretHolder.js
[++++] /zpool0/trunk/binaries/data/mods/public/simulation/components/TurretHolder.js
[ 288] 				this.LeaveTurret(entity)
[ 288] 				this.LeaveTurret(entity);

binaries/data/mods/public/simulation/components/TurretHolder.js
[ 288] »   »   »   »   this.LeaveTurret(entity)
**** JSHintBear [Section: JS | Severity: NORMAL] ****
!    ! Missing semicolon.
Executing section cli...

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

Stan added a subscriber: Stan.Aug 21 2020, 10:29 AM
Stan added inline comments.
source/graphics/MapWriter.cpp
368 ↗(On Diff #13126)
372 ↗(On Diff #13126)

Do you need the cast?

source/simulation2/components/ICmpTurretHolder.cpp
48 ↗(On Diff #13126)
Freagarach updated this revision to Diff 13264.Aug 21 2020, 4:32 PM
Freagarach marked 2 inline comments as done.

Use references.

source/graphics/MapWriter.cpp
372 ↗(On Diff #13126)

Other places seem to do it as well (not only GarrisonHolder, which I wrote also).

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

builderr-debug-macos.txt
../../../source/simulation2/scripting/JSInterface_Simulation.cpp:155:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        CFixedVector2D(-halfSize.X, -halfSize.Y),
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
In file included from ../../../source/graphics/tests/test_Camera.cpp:17:
/Users/wfg/Jenkins/workspace/macos-differential/source/graphics/tests/test_Camera.h:168:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        CVector3D(-101.0f, -101.0f, 101.0f),
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
In file included from ../../../source/simulation2/tests/test_SerializeTemplates.cpp:17:
/Users/wfg/Jenkins/workspace/macos-differential/source/simulation2/tests/test_SerializeTemplates.h:39:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        3, 0, 1, 4, 1, 5
                        ^~~~~~~~~~~~~~~~
                        {               }
1 warning generated.
builderr-release-macos.txt
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libnetwork.a(precompiled.o) has no symbols
../../../source/simulation2/scripting/JSInterface_Simulation.cpp:155:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        CFixedVector2D(-halfSize.X, -halfSize.Y),
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libsimulation2.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libengine.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libgraphics.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libatlas.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libgui.a(precompiled.o) has no symbols
In file included from ../../../source/graphics/tests/test_Camera.cpp:17:
/Users/wfg/Jenkins/workspace/macos-differential/source/graphics/tests/test_Camera.h:168:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        CVector3D(-101.0f, -101.0f, 101.0f),
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
In file included from ../../../source/simulation2/tests/test_SerializeTemplates.cpp:17:
/Users/wfg/Jenkins/workspace/macos-differential/source/simulation2/tests/test_SerializeTemplates.h:39:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        3, 0, 1, 4, 1, 5
                        ^~~~~~~~~~~~~~~~
                        {               }
1 warning generated.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1388/display/redirect

wraitii accepted this revision.Nov 6 2020, 5:26 PM

Looks good, runs. Maybe add comments to ICmpTurretHolder.

This revision is now accepted and ready to land.Nov 6 2020, 5:26 PM

What kind of comments?

Angen added inline comments.Nov 6 2020, 5:32 PM
binaries/data/mods/public/simulation/components/TurretHolder.js
288 ↗(On Diff #13264)

^ :)

Freagarach updated this revision to Diff 13767.Nov 6 2020, 6:56 PM
Freagarach marked 2 inline comments as done.
  • Comments.
  • Linter.
Vulcan added a comment.Nov 6 2020, 7:04 PM

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

builderr-debug-gcc6.txt
../../../source/gui/ObjectTypes/CMiniMap.cpp:18:2: error: invalid preprocessing directive #include
 #include "precompiled.h"
  ^~~~~~~
../../../source/gui/ObjectTypes/CMiniMap.cpp:20:2: error: invalid preprocessing directive #include
 #include "CMiniMap.h"
  ^~~~~~~
../../../source/gui/ObjectTypes/CMiniMap.cpp:22:2: error: invalid preprocessing directive #include
 #include "graphics/GameView.h"
  ^~~~~~~
../../../source/gui/ObjectTypes/CMiniMap.cpp:23:2: error: invalid preprocessing directive #include
 #include "graphics/LOSTexture.h"
  ^~~~~~~
../../../source/gui/ObjectTypes/CMiniMap.cpp:24:2: error: invalid preprocessing directive #include
 #include "graphics/MiniPatch.h"
  ^~~~~~~
../../../source/gui/ObjectTypes/CMiniMap.cpp:25:2: error: invalid preprocessing directive #include
 #include "graphics/Terrain.h"
  ^~~~~~~
../../../source/gui/ObjectTypes/CMiniMap.cpp:26:2: error: invalid preprocessing directive #include
 #include "graphics/TerrainTextureEntry.h"
  ^~~~~~~
../../../source/gui/ObjectTypes/CMiniMap.cpp:27:2: error: invalid preprocessing directive #include
 #include "graphics/TerrainTextureManager.h"
  ^~~~~~~
../../../source/gui/ObjectTypes/CMiniMap.cpp:28:2: error: invalid preprocessing directive #include
 #include "graphics/TerritoryTexture.h"
  ^~~~~~~
../../../source/gui/ObjectTypes/CMiniMap.cpp:29:2: error: invalid preprocessing directive #include
 #include "gui/CGUI.h"
  ^~~~~~~
../../../source/gui/ObjectTypes/CMiniMap.cpp:30:2: error: invalid preprocessing directive #include
 #include "gui/GUIManager.h"
  ^~~~~~~
../../../source/gui/ObjectTypes/CMiniMap.cpp:31:2: error: invalid preprocessing directive #include
 #include "gui/GUIMatrix.h"
  ^~~~~~~
../../../source/gui/ObjectTypes/CMiniMap.cpp:32:2: error: invalid preprocessing directive #include
 #include "lib/bits.h"
  ^~~~~~~
../../../source/gui/ObjectTypes/CMiniMap.cpp:33:2: error: invalid preprocessing directive #include
 #include "lib/external_libraries/libsdl.h"
  ^~~~~~~
../../../source/gui/ObjectTypes/CMiniMap.cpp:34:2: error: invalid preprocessing directive #include
 #include "lib/ogl.h"
  ^~~~~~~
../../../source/gui/ObjectTypes/CMiniMap.cpp:35:2: error: invalid preprocessing directive #include
 #include "lib/timer.h"
  ^~~~~~~
../../../source/gui/ObjectTypes/CMiniMap.cpp:36:2: error: invalid preprocessing directive #include
 #include "ps/ConfigDB.h"
  ^~~~~~~
../../../source/gui/ObjectTypes/CMiniMap.cpp:37:2: error: invalid preprocessing directive #include
 #include "ps/Filesystem.h"
  ^~~~~~~
../../../source/gui/ObjectTypes/CMiniMap.cpp:38:2: error: invalid preprocessing directive #include
 #include "ps/Game.h"
  ^~~~~~~
../../../source/gui/ObjectTypes/CMiniMap.cpp:39:2: error: invalid preprocessing directive #include
 #include "ps/GameSetup/Config.h"
  ^~~~~~~
../../../source/gui/ObjectTypes/CMiniMap.cpp:40:2: error: invalid preprocessing directive #include
 #include "ps/Profile.h"
  ^~~~~~~
../../../source/gui/ObjectTypes/CMiniMap.cpp:41:2: error: invalid preprocessing directive #include
 #include "ps/World.h"
  ^~~~~~~
../../../source/gui/ObjectTypes/CMiniMap.cpp:42:2: error: invalid preprocessing directive #include
 #include "ps/XML/Xeromyces.h"
  ^~~~~~~
../../../source/gui/ObjectTypes/CMiniMap.cpp:43:2: error: invalid preprocessing directive #include
 #include "renderer/Renderer.h"
  ^~~~~~~
../../../source/gui/ObjectTypes/CMiniMap.cpp:44:2: error: invalid preprocessing directive #include
 #include "renderer/RenderingOptions.h"
  ^~~~~~~
../../../source/gui/ObjectTypes/CMiniMap.cpp:45:2: error: invalid preprocessing directive #include
 #include "renderer/WaterManager.h"
  ^~~~~~~
../../../source/gui/ObjectTypes/CMiniMap.cpp:46:2: error: invalid preprocessing directive #include
 #include "scriptinterface/ScriptInterface.h"
  ^~~~~~~
../../../source/gui/ObjectTypes/CMiniMap.cpp:47:2: error: invalid preprocessing directive #include
 #include "simulation2/Simulation2.h"
  ^~~~~~~
../../../source/gui/ObjectTypes/CMiniMap.cpp:48:2: error: invalid preprocessing directive #include
 #include "simulation2/components/ICmpMinimap.h"
  ^~~~~~~
../../../source/gui/ObjectTypes/CMiniMap.cpp:49:2: error: invalid preprocessing directive #include
 #include "simulation2/helpers/Los.h"
  ^~~~~~~
../../../source/gui/ObjectTypes/CMiniMap.cpp:50:2: error: invalid preprocessing directive #include
 #include "simulation2/system/ParamNode.h"
  ^~~~~~~
../../../source/gui/ObjectTypes/CMiniMap.cpp:52:2: error: invalid preprocessing directive #include
 #include <cmath>
  ^~~~~~~
../../../source/gui/ObjectTypes/CMiniMap.cpp:58: confused by earlier errors, bailing out
make[1]: *** [obj/gui_Debug/CMiniMap.o] Error 1
make: *** [gui] Error 2

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

Vulcan added a comment.Nov 6 2020, 7:07 PM

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

builderr-debug-macos.txt
../../../source/simulation2/scripting/JSInterface_Simulation.cpp:155:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        CFixedVector2D(-halfSize.X, -halfSize.Y),
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
In file included from ../../../source/graphics/tests/test_Camera.cpp:17:
/Users/wfg/Jenkins/workspace/macos-differential/source/graphics/tests/test_Camera.h:168:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        CVector3D(-101.0f, -101.0f, 101.0f),
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
In file included from ../../../source/simulation2/tests/test_SerializeTemplates.cpp:17:
/Users/wfg/Jenkins/workspace/macos-differential/source/simulation2/tests/test_SerializeTemplates.h:39:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        3, 0, 1, 4, 1, 5
                        ^~~~~~~~~~~~~~~~
                        {               }
1 warning generated.
builderr-release-macos.txt
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libnetwork.a(precompiled.o) has no symbols
../../../source/simulation2/scripting/JSInterface_Simulation.cpp:155:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        CFixedVector2D(-halfSize.X, -halfSize.Y),
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libsimulation2.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libengine.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libgraphics.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libatlas.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libgui.a(precompiled.o) has no symbols
In file included from ../../../source/graphics/tests/test_Camera.cpp:17:
/Users/wfg/Jenkins/workspace/macos-differential/source/graphics/tests/test_Camera.h:168:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        CVector3D(-101.0f, -101.0f, 101.0f),
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
In file included from ../../../source/simulation2/tests/test_SerializeTemplates.cpp:17:
/Users/wfg/Jenkins/workspace/macos-differential/source/simulation2/tests/test_SerializeTemplates.h:39:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        3, 0, 1, 4, 1, 5
                        ^~~~~~~~~~~~~~~~
                        {               }
1 warning generated.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1764/display/redirect

This revision was landed with ongoing or failed builds.Nov 11 2020, 8:40 PM
This revision was automatically updated to reflect the committed changes.