bugfix(behavior): Fix unexpected propaganda influence from contained units with direct Propaganda Tower behavior modules #1609
Conversation
…units with direct Propaganda Tower behavior modules
| Object *getContainedBySpecialOverlordStyle(); ///< Get parent container object if its contain module is Overlord style. | ||
| const Object *getContainedBySpecialOverlordStyle() const; ///< Get parent container object if its contain module is Overlord style. |
There was a problem hiding this comment.
Is the Object class the best place for these functions? It feels backwards to put specific behaviour here (despite most of the code base already breaking this principle).
There was a problem hiding this comment.
Do you have suggestion where to put it?
There was a problem hiding this comment.
Ideally the container should be aware of whether its occupants are exposed / visible (I believe isEnclosingContainerFor might typically be used for this purpose). This would then allow all non-enclosed contained objects to provide their Propaganda effects, such as infantry in a Firebase if the PropagandaTowerBehavior module were to be added to them.
There was a problem hiding this comment.
I do not know what you mean. Please create a new Pull if there is another different fix proposal.
There was a problem hiding this comment.
Creating endless fix proposals is inefficient over simply discussing the solution. Which parts are unclear?
If #1545 is not generic enough then I suggest this change is not either. The ideal solution also supports infantry with PropagandaTowerBehavior modules in Firebases.
There was a problem hiding this comment.
Ok, how would the test for that look like?
There was a problem hiding this comment.
You would copy any PropagandaTowerBehavior module from an applicable China object (e.g. from ChinaTankOverlordPropagandaTower in ChinaVehicle.ini) and add it to any USA infantry unit (e.g. to AmericaInfantryRanger in AmericaInfantry.ini). Also ensure the module name ModuleTag_XX is unique.
Then start a new game as USA and build the respective infantry unit, which should now be producing an observable Propaganda effect. Build a Firebase and garrison the infantry unit within the Firebase. If the Propaganda effect turns off when the infantry unit is garrisoned, then the solution is inadequate and we lose functionality from retail.
There was a problem hiding this comment.
I meant how would the test in C++ look like. The if condition for testing the container.
There was a problem hiding this comment.
There is a field on the GarrisonContain module - IsEnclosingContainer that is set to false for the Firebase (and dynamically returned for OverlordContain::isEnclosingContainerFor). Perhaps similar logic is needed in this case?
This fix is an alternative to #1545 and fixes the unexpected propaganda influence from contained units with direct Propaganda Tower behavior modules.
TODO