bugfix: Do not show veterancy effects for dead units

[Stubbjax]

This change prevents veterancy effects from showing for dead units. It will also save running some redundant upgrade logic once retail compatibility is dropped.

Issue Demonstrations

DEAD_VET_1.mp4

DEAD_VET_2.mp4

[xezon]

That should do:

#if RETAIL_COMPATIBLE_CRC
	doAnimation &= isEffectivelyDead();
#endif

[Stubbjax]

Does it really matter when they're effectively the same thing and this one is deprecated? Having them the same highlights that both compatible and non-compatible logic is identical and the only difference is when it is called.

[xezon]

My motivation here was to get rid of the return in the middle of the function.

[Stubbjax]

Why is a return undesirable?

[xezon]

Breakdown, courtesy of Chat Gippity

Returning early from a function (i.e., having multiple return statements, especially in the middle of long or complex C++ functions) is often debated. What speaks against it are mainly concerns around readability, maintainability, and correctness in complex code. Here are the main arguments.

---

1. Loss of a clear control-flow narrative

In long or complex functions, early returns can fragment the logic.

• You no longer have a single “entry → processing → exit” flow.
• Readers must mentally track many possible exit points.
• Understanding which code runs in which situations becomes harder.

This is especially problematic when:

• The function has multiple phases (setup, processing, cleanup).
• Later code depends subtly on earlier conditions.

---

2. Harder reasoning about invariants and state

Complex functions often establish invariants:

// After this point, x and y are valid and synchronized

If you return early:

• Some invariants may only hold on some paths.
• You must verify that every return happens before or after invariants are established.
• Future modifications can easily violate assumptions.

This increases the cognitive load when modifying the function later.

---

3. Resource management and cleanup risks (without RAII)

Historically (and still in some legacy code):

• Multiple return points made it easy to forget cleanup:

  • Memory deallocation
  • Unlocking mutexes
  • Closing files

Example risk:

lock(m);
if (error) return; // mutex never unlocked

Modern C++ mitigates this with RAII, but:

• Legacy code may not use RAII consistently.
• Mixing RAII and manual cleanup is still error-prone.

---

4. Debugging and instrumentation complexity

Multiple return points complicate:

• Placing breakpoints at “the end” of a function
• Logging exit conditions consistently
• Adding profiling or tracing later

With one exit point, you can:

• Log once
• Validate postconditions
• Centralize error reporting

---

5. Increased risk when modifying or extending the function

In long functions, future changes are common:

• Adding new cleanup logic
• Adding metrics or tracing
• Enforcing postconditions

With many early returns:

• Every return may need updating
• Missing one return can introduce subtle bugs

This scales poorly as complexity grows.

---

6. Violates some team or project conventions

Many codebases explicitly prefer:

• “Single exit point” for non-trivial functions

• Or early returns only for:

  • Guard clauses at the top
  • Simple error handling

Violating conventions reduces consistency and codebase readability.

---

7. Encourages overly large functions

Early returns can mask deeper design issues:

• A function doing too much
• Many responsibilities and states

Refactoring into smaller functions often:

• Makes early returns unnecessary
• Improves readability and testability

---

Balanced perspective (important)

Early returns are not inherently bad.

They are generally good when:

• Used as guard clauses at the top
• The function is short and simple
• They improve readability by avoiding deep nesting

They are more questionable when:

• The function is long and multi-phase
• There are resources or invariants to maintain
• The function evolves over time

---

Practical guideline

A commonly accepted compromise in modern C++:

Use early returns for validation and error handling at the top.
Avoid mid-function returns in long, stateful, or multi-phase functions.

If mid-function returns feel necessary:

• Consider extracting a helper function
• Or restructuring into clearer phases

[Stubbjax]

Not many if any of those concerns apply here. The method is clearly separated into two blocks - logic and then animation; placing a return between the two blocks is reasonable. Matching the code within both precompiler directives aids in understanding that it is the same logic with only the placement being the difference.

[xezon]

Can add TheSuperHackers comment because it is user facing, even if obvious what it does.

[Caball009]

Does this require retail compatibility? It looks like this only disables sound and visuals locally.

[Stubbjax]

Technically not. The idea is that this code gets removed when retail compatibility is dropped, so leaving this here will make it easier to keep track of.

[xezon]

What is the status here?

[Stubbjax]

What is the status here?

Are there any requested changes?

[xezon]

There were no commit pushes and no comments after the last review comments so the work is not concluded.

[Stubbjax]

There were no commit pushes and no comments after the last review comments so the work is not concluded.

Oh, my comment was somehow pending but not submitted.