bugfix: Allow immediate resumed construction of buildings if the existing builder dies

[Stubbjax]

This change allows immediate resumed construction of buildings if the existing builder dies. In the retail game, the player must await the SlowDeathBehavior module's DestructionDelay / SinkDelay before construction can resume. This delay is up to 2100ms for USA and China Dozers with a fixed 3000ms delay for GLA Workers.

Before

The player must wait for the original builder's SlowDeathBehavior delay before construction can resume with another builder

DELAYED_RESUME.mp4

After

The player is free to resume construction the instant the existing builder dies

INSTANT_RESUME.mp4

[Skyaero42]

It don't think this is a bug but a design choice. "Fixing" this will have a (minor) impact on balance. The 2.1 second delay or not of building e.g. an EMP patriot can be the decisive factor in winning or losing early game.

[Stubbjax]

It don't think this is a bug but a design choice. "Fixing" this will have a (minor) impact on balance. The 2.1 second delay or not of building e.g. an EMP patriot can be the decisive factor in winning or losing early game.

Why is it not a bug? A player typically expects to be able to immediately resume a scaffold that is not under active construction. Construction can be immediately resumed in all other scenarios, such as if the builder becomes disabled, unmanned, captured, is stopped, is redirected, etc. Why not death?

A designer also expects the SlowDeathBehavior module's destruction or sink delay to be purely aesthetic without having any bearing on gameplay logic. A designer might want a fancy 10-second death effect without it coming with the weird and unexpected side effect of blocking construction for 10 seconds.

Fixing this will indeed have (very minor) gameplay implications, like many other bug fixes. But that does not invalidate its status as a bug. Ideally we don't want games to be decided due to an arbitrary delay on input / unit responsiveness.

[Skyaero42]

Why is it not a bug? A player typically expects to be able to immediately resume a scaffold that is not under active construction.

It may not be a bug, but an intentional game mechanic, but I don't know that for certain, only the EA Developers at the time know. I could imagine that this is a punishment/reward form: delaying the construction of the building.

Construction can be immediately resumed in all other scenarios, such as if the builder becomes disabled, unmanned, captured, is stopped, is redirected, etc. Why not death?

And this would be a fair point why it could be bug instead.

A designer also expects the SlowDeathBehavior module's destruction or sink delay to be purely aesthetic without having any bearing on gameplay logic. A designer might want a fancy 10-second death effect without it coming with the weird and unexpected side effect of blocking construction for 10 seconds.

I would agree, yet the implementation by EA of SlowDeathBehavior is not designed that way. It contains death logic - as the actual dead of the unit is called at the end of the behaviour through OnDie.

Which adds a concern to me that the provided solution may be a hack fix: The real problem would lie n animation behaviour containing logic behaviour. The proper solution would be a rewrite of the SlowDeathBehaviour module - which is quite the task.

Fixing this will indeed have (very minor) gameplay implications, like many other bug fixes. But that does not invalidate its status as a bug. Ideally we don't want games to be decided due to an arbitrary delay on input / unit responsiveness.

So far, we have barely touched gameplay bugs that affect gameplay. Most are still working on foundation, stability and Gentool porting.

I would have rather seen an issue opened first for this PR - and then discussed the potential impact on gameplay and the gaming community. There will be many many more of these small tweaks that will impact the game - even minor. We need to tread carefully here and ensure that any changes are in line with what the (majority of) player community expects and wishes, identify where the controversies are and provide configurability for those.

[Stubbjax]

It may not be a bug, but an intentional game mechanic, but I don't know that for certain, only the EA Developers at the time know. I could imagine that this is a punishment/reward form: delaying the construction of the building.

I'm not sure random delays on unit responsiveness can be considered an intentional game mechanic by any stretch of the imagination. Regardless, we should be careful about projecting too much intent onto the original developers as a means of justification.

I would agree, yet the implementation by EA of SlowDeathBehavior is not designed that way. It contains death logic - as the actual dead of the unit is called at the end of the behaviour through OnDie.

In this case, we are not concerned with the actual dead state of the unit - we are concerned with the effectively dead state. This is the entire reason for the isEffectivelyDead function's existence - so that any active references to a dying object do not influence gameplay or allow / prevent actions from being taken with / against them.

Which adds a concern to me that the provided solution may be a hack fix: The real problem would lie n animation behaviour containing logic behaviour. The proper solution would be a rewrite of the SlowDeathBehaviour module - which is quite the task.

How and why would that be the proper solution? This fix follows the same pattern you'll find across the codebase. I'd suggest searching for references to isEffectivelyDead to see the extent of its coverage. You can intuit the original intent regarding this case in the action manager with the reference check; the oversight is that it is not checking its isEffectivelyDead state along with it, which is typically done everywhere else. The very same check is done for the selected builder unit a few lines up in the function.

So far, we have barely touched gameplay bugs that affect gameplay. Most are still working on foundation, stability and Gentool porting.

I'm not sure what your point is here. The majority of the bugs I have tackled thus far have been gameplay bugs. Other developers are free to work on whatever they like, but these are the issues I find most interesting and motivating to work on. I'm not sure there are even many more stability / GenTool ports left to do that aren't already in progress.

I would have rather seen an issue opened first for this PR - and then discussed the potential impact on gameplay and the gaming community. There will be many many more of these small tweaks that will impact the game - even minor. We need to tread carefully here and ensure that any changes are in line with what the (majority of) player community expects and wishes, identify where the controversies are and provide configurability for those.

While I would tend to agree for larger and more impactful changes, I think this is overkill for a minor bugfix that makes an objective improvement to the game. A level of common sense needs to apply or we'll get stuck in the weeds arguing over the colour of Colonel Burton's pants.

Such discussions are also free to be had in the respective PR, which provides the benefit of allowing testing of the change. Please be assured that I tread incredibly carefully. If you firmly believe this fix is likely to invoke the wrath of the Angry Mob (I personally believe the odds are infinitesimally small) then I will gladly adjust the RETAIL_COMPATIBLE_CRC condition to RETAIL_COMPATIBLE_BUG.

[xezon]

I agree that fixing this issue is the logical correct behavior. It is a minor controversial bug. I expect that most players will expect that they can continue building as soon as the Worker dies. It definitely needs to be behind RETAIL_COMPATIBLE_CRC. It can be behind RETAIL_COMPATIBLE_BUG (PRESERVE_RETAIL_BEHAVIOR) on top of that to prevent immediate controversy.