Skip to content

bugfix(partition): Fix inconsistent mine collision behaviour#2208

Merged
xezon merged 3 commits intoTheSuperHackers:mainfrom
Stubbjax:improve-circular-partition-cell-coverage
Feb 27, 2026
Merged

bugfix(partition): Fix inconsistent mine collision behaviour#2208
xezon merged 3 commits intoTheSuperHackers:mainfrom
Stubbjax:improve-circular-partition-cell-coverage

Conversation

@Stubbjax
Copy link

Fixes #24 from the patch repository

This change fixes inconsistent mine collision detection by replacing the partition manager's circle fill algorithm with a more precise algorithm. The new logic is placed behind a new RETAIL_COMPATIBLE_CIRCLE_FILL_ALGORITHM preprocessor directive to help isolate the change as it is potentially slightly less performant.

The retail circle fill logic - an implementation of the midpoint circle algorithm - does not accurately cover the cells occupied by spherical or cylindrical geometry. This leads to various collision inaccuracies throughout the game, one of which is the infamous mine behaviour described in #24. Various collision logic is only processed within partition cells that are considered occupied - the partition cells act as a 'broad phase' to determine which geometry then needs to be checked for overlaps.

It's particularly important to note that objects with a geometry radius >= 20 and < 40 (the most common range) only cover a single partition cell, for example Tunnel Networks (radius: 25) and Stinger Sites (radius: 30). This can be seen in the video below. A blue/green square indicates an occupied cell.

SINGLE_CELL.mp4

Mines happen to fit right within this range with a radius of 30, and therefore only end up covering a single cell. This is a problem as most units happen to be smaller than a single partition cell, and can end up bypassing adjacent occupied cells altogether. Below is an example of this happening.

DODGER.mp4

Below is a demonstration of the difference in cell coverage in retail (left, blue) vs this change (right, yellow) at different radii. It should be apparent that retail cell coverage becomes more inaccurate as geometry radii increases towards each multiple of the partition cell size (40, 80, 120, etc.), whereas the new algorithm is always perfectly accurate. Note that radii < 20 uses a separate dedicated 'small geometry' algorithm which is 100% accurate.

GRID_COMP.mp4

And below is a direct comparison between retail mine cell coverage and the new mine cell coverage.

Before

Mine cell coverage using the retail algorithm

image

After

Mine cell coverage using the new algorithm

image

And some real gameplay demonstrations for good measure.

Before

The Technical skips over several mines, allowing the terrorists to destroy the War Factory

BAD_MINES.mp4

After

The Technical and terrorists explode on the mines as expected

GOOD_MINES.mp4

@Stubbjax Stubbjax self-assigned this Jan 28, 2026
@Stubbjax Stubbjax added Bug Something is not working right, typically is user facing Major Severity: Minor < Major < Critical < Blocker Performance Is a performance concern Gen Relates to Generals ZH Relates to Zero Hour NoRetail This fix or change is not applicable with Retail game compatibility labels Jan 28, 2026
@greptile-apps
Copy link

greptile-apps bot commented Jan 28, 2026

Greptile Summary

This PR fixes inconsistent mine collision detection by replacing the retail circle fill algorithm with a precise circle-rectangle overlap test. The new algorithm eliminates gaps in partition cell coverage for objects with radii between 20-40 units (like mines with radius 30), ensuring units can no longer bypass adjacent occupied cells.

  • Implemented doCircleFillPrecise() using standard AABB collision detection (finds closest point on rectangle to circle center, checks if within radius)
  • Added RETAIL_COMPATIBLE_CIRCLE_FILL_ALGORITHM preprocessor directive (defaults to enabled) to maintain backward compatibility
  • Applied changes to both Generals and GeneralsMD codebases to fix vanilla and Zero Hour expansion
  • The implementation correctly handles edge cases (null cells, boundary conditions, negative coordinates)
  • Trade-off: slightly less performant for large radii (O(r²) vs O(r)), but accuracy improvement is significant for common object sizes

Confidence Score: 5/5

  • This PR is safe to merge with high confidence - well-tested bugfix with backward compatibility controls
  • The implementation uses a standard, proven algorithm for circle-rectangle collision detection. Code is well-structured with appropriate null checks and boundary handling. The change is properly isolated behind a preprocessor directive allowing fallback to retail behavior if needed. Extensive testing evidenced by videos and visual demonstrations in the PR description confirm the fix works as intended.
  • No files require special attention

Important Files Changed

Filename Overview
Core/GameEngine/Include/Common/GameDefines.h Added RETAIL_COMPATIBLE_CIRCLE_FILL_ALGORITHM preprocessor directive to control algorithm selection
Generals/Code/GameEngine/Source/GameLogic/Object/PartitionManager.cpp Implemented precise circle-cell overlap detection using standard AABB algorithm; conditionally enabled via preprocessor directive
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/PartitionManager.cpp Implemented precise circle-cell overlap detection (Zero Hour version)

Last reviewed commit: 10ec115

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

@xezon
Copy link

xezon commented Feb 2, 2026

I did not look at the code yet but the PRESENTATION and DILIGENCE is WORLD CLASS! Massive APPLAUSE 👏👏👏

@xezon xezon added Buff Makes a thing more powerful China Affects China faction labels Feb 3, 2026
Copy link

@xezon xezon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementation looks very good too.

Could be refactored a little.

Needs rebase.

@Stubbjax Stubbjax force-pushed the improve-circular-partition-cell-coverage branch from d9274de to dc18156 Compare February 27, 2026 07:16
@xezon xezon changed the title bugfix: Fix inconsistent mine collision behaviour bugfix(partition): Fix inconsistent mine collision behaviour Feb 27, 2026
@xezon xezon merged commit bb193fd into TheSuperHackers:main Feb 27, 2026
25 checks passed
@Stubbjax Stubbjax deleted the improve-circular-partition-cell-coverage branch February 28, 2026 00:08
CookieLandProjects pushed a commit to CookieLandProjects/CLP_AI that referenced this pull request Feb 28, 2026
Okladnoj pushed a commit to Okladnoj/GeneralsGameCode that referenced this pull request Mar 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Buff Makes a thing more powerful Bug Something is not working right, typically is user facing China Affects China faction Gen Relates to Generals Major Severity: Minor < Major < Critical < Blocker NoRetail This fix or change is not applicable with Retail game compatibility Performance Is a performance concern ZH Relates to Zero Hour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Investigate China mines issues, where units can drive or walk through or over mines without taking damage

2 participants