Page MenuHomeWildfire Games

Allow units to react when their path is obstructed / Change attack orders to not be forced
AbandonedPublic

Authored by wraitii on Dec 27 2016, 1:53 PM.

Details

Reviewers
None
Trac Tickets
#4340
Summary

Ported from 4340. See comments there, this needs changes

Test Plan

Am interested in comments on the new unit behaviour and patch review to an extent.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 40
Build 65: Vulcan BuildJenkins
Build 64: arc lint + arc unit

Event Timeline

wraitii updated this revision to Diff 44.Dec 27 2016, 1:53 PM
wraitii retitled this revision from to Allow units to react when their path is obstructed / Change attack orders to not be forced.
wraitii updated this object.
wraitii edited the test plan for this revision. (Show Details)
wraitii set the repository for this revision to rP 0 A.D. Public Repository.
Vulcan added a subscriber: Vulcan.Dec 27 2016, 2:26 PM

Build has FAILED

Build (release)...
../../../source/gui/CChart.cpp:38:41: warning: unused parameter ‘Message’ [-Wunused-parameter]
 void CChart::HandleMessage(SGUIMessage& Message)
                                         ^
../../../source/lib/tex/tex_png.cpp: In member function ‘virtual Status TexCodecPng::encode(Tex*, DynArray*) const’:
../../../source/lib/tex/tex_png.cpp:309:9: warning: variable ‘ret’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Wclobbered]
  Status ret = ERR::FAIL;
         ^
Build (debug)...
../../../source/gui/CChart.cpp:38:41: warning: unused parameter ‘Message’ [-Wunused-parameter]
 void CChart::HandleMessage(SGUIMessage& Message)
                                         ^
Running debug tests...
Build (release)...
../../../source/simulation2/components/CCmpUnitMotion.cpp:350:45: warning: unused parameter ‘paramNode’ [-Wunused-parameter]
  virtual void Deserialize(const CParamNode& paramNode, IDeserializer& deserialize)
                                             ^
../../../source/simulation2/components/CCmpUnitMotion.cpp:350:71: warning: unused parameter ‘deserialize’ [-Wunused-parameter]
  virtual void Deserialize(const CParamNode& paramNode, IDeserializer& deserialize)
                                                                       ^
../../../source/simulation2/components/CCmpUnitMotion.cpp: In instantiation of ‘void CCmpUnitMotion::SerializeCommon(S&) [with S = ISerializer]’:
../../../source/simulation2/components/CCmpUnitMotion.cpp:347:28:   required from here
../../../source/simulation2/components/CCmpUnitMotion.cpp:314:26: warning: unused parameter ‘serialize’ [-Wunused-parameter]
  void SerializeCommon(S& serialize)
                          ^
Build (debug)...
../../../source/simulation2/components/CCmpUnitMotion.cpp:350:45: warning: unused parameter ‘paramNode’ [-Wunused-parameter]
  virtual void Deserialize(const CParamNode& paramNode, IDeserializer& deserialize)
                                             ^
../../../source/simulation2/components/CCmpUnitMotion.cpp:350:71: warning: unused parameter ‘deserialize’ [-Wunused-parameter]
  virtual void Deserialize(const CParamNode& paramNode, IDeserializer& deserialize)
                                                                       ^
../../../source/simulation2/components/CCmpUnitMotion.cpp: In instantiation of ‘void CCmpUnitMotion::SerializeCommon(S&) [with S = ISerializer]’:
../../../source/simulation2/components/CCmpUnitMotion.cpp:347:28:   required from here
../../../source/simulation2/components/CCmpUnitMotion.cpp:314:26: warning: unused parameter ‘serialize’ [-Wunused-parameter]
  void SerializeCommon(S& serialize)
                          ^
Running debug tests...
./test_dbg: error while loading shared libraries: libmozjs38-ps-debug.so: cannot open shared object file: No such file or directory
Build (release)...
In file included from ../../../source/simulation2/components/CCmpPathfinder_Common.h:39:0,
                 from ../../../source/simulation2/components/CCmpPathfinder.cpp:25:
../../../source/simulation2/helpers/LongPathfinder.h:266:15: error: ‘mutex’ in namespace ‘std’ does not name a type
  mutable std::mutex m_Mutex;
               ^
make[1]: *** [obj/simulation2_Release/CCmpPathfinder.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [simulation2] Error 2
make: *** Waiting for unfinished jobs....
Build (release)...
Build (debug)...
Running debug tests...
./test_dbg: error while loading shared libraries: libmozjs38-ps-debug.so: cannot open shared object file: No such file or directory

Link to build: http://jw:8080/job/phabricator/23/
See console output for more information: http://jw:8080/job/phabricator/23/console

wraitii updated this revision to Diff 55.Dec 28 2016, 12:56 PM

Reupload.

Build has FAILED

Updating workspaces
Build (release)...
../../../source/gui/CChart.cpp:38:41: warning: unused parameter ‘Message’ [-Wunused-parameter]
 void CChart::HandleMessage(SGUIMessage& Message)
                                         ^
../../../source/lib/tex/tex_png.cpp: In member function ‘virtual Status TexCodecPng::encode(Tex*, DynArray*) const’:
../../../source/lib/tex/tex_png.cpp:309:9: warning: variable ‘ret’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Wclobbered]
  Status ret = ERR::FAIL;
         ^

Link to build: http://jw:8080/job/phabricator/30/
See console output for more information: http://jw:8080/job/phabricator/30/console

wraitii planned changes to this revision.Jan 19 2017, 6:03 PM
bb added a subscriber: bb.Feb 7 2017, 4:46 PM
bb added inline comments.
binaries/data/mods/public/simulation/components/UnitAI.js
1871

When running some tests with big masses of units, units still wonder around a bit after they changed target. I guess this is due to that the obstructing unit moved since the obstruction message was send. Maybe it would be better not attacking the obstructing unit explicitly, but using the "FindNewTarget" function to select and attack a new entity. I didn't test what effect this has on performance.

Also it would be good to add this same check in the "ATTACK.CHASING" state.

5147

Could you please explain why this change is necessary, to me it seems counterintuitive to let the unitAI only decide what to attack, when explicitly attacking a unit with an attack command (i.o. not attack-move or something).

6083

These code changes only change the outcome when preferredclasses are specified in template. Most units don't have them and imo they seem a bit deprecated, so I am fine with these changes, only we nuke support for preferredclasses with these changes even further and if we want to make better support for them, or put in a new system, these changes would need to be reverted.

elexis changed the visibility from "All Users" to "Public (No Login Required)".Mar 25 2017, 6:03 PM
fatherbushido updated the Trac tickets for this revision.May 30 2017, 10:27 PM
fatherbushido added a subscriber: fatherbushido.
wraitii abandoned this revision.Oct 28 2017, 7:10 PM

We'll see after D13