Skip to content

Commit 6a4e5c1

Browse files
committed
revert: Keep 3 GameClient files in Generals/MD to avoid PR conflicts
Reverting unification to avoid conflicts with: - PR TheSuperHackers#654 (tomsons26): Cleans up inconsistencies - PR TheSuperHackers#688 (tomsons26): Backports various things from ZH Files reverted: Color.h, Mouse.h, Color.cpp
1 parent 469c4d7 commit 6a4e5c1

22 files changed

+6471
-9
lines changed

ANALYSIS_METHODOLOGY.md

Lines changed: 280 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,280 @@
1+
# Conflict Analysis Methodology
2+
3+
## Overview
4+
5+
This document explains how the PR conflict analysis was performed and how to reproduce it.
6+
7+
## Tools Used
8+
9+
1. **GitHub CLI (`gh`)** - For fetching PR data from GitHub API
10+
2. **Git** - For comparing branch differences
11+
3. **Python 3** - For data processing and analysis
12+
13+
## Analysis Process
14+
15+
### Step 1: Fetch Open PRs
16+
17+
```bash
18+
gh pr list --limit 25 --json number,title,state,files,author --repo TheSuperHackers/GeneralsGameCode
19+
```
20+
21+
This fetches:
22+
- PR number and title
23+
- List of all files changed in each PR
24+
- Author information
25+
- Current state (open/closed)
26+
27+
### Step 2: Get Files Changed in Unification Branches
28+
29+
For each unification branch:
30+
31+
```bash
32+
git diff --name-only main..bobtista/unify-common
33+
git diff --name-only main..bobtista/unify-gameclient
34+
git diff --name-only main..bobtista/unify-gamelogic
35+
git diff --name-only main..bobtista/unify-network
36+
```
37+
38+
This gives us the complete list of files being moved/changed in each unification effort.
39+
40+
### Step 3: Find Overlapping Files
41+
42+
For each unification branch, find files that appear in BOTH:
43+
- The unification branch changes
44+
- Any open PR's changes
45+
46+
If a file appears in both, it's a **potential conflict**.
47+
48+
### Step 4: Categorize by Risk Level
49+
50+
**🟢 Low Risk**: Only `CMakeLists.txt` files conflict
51+
- These are routine build configuration conflicts
52+
- Easy to merge by combining both sets of changes
53+
54+
**🟡 Medium Risk**: 1-3 files conflict
55+
- Usually straightforward to resolve
56+
- May require reviewing the specific changes
57+
58+
**🔴 High Risk**: 4+ files conflict
59+
- Significant overlap with other work
60+
- Requires coordination with PR author
61+
- May need to merge one PR first, then rebase the other
62+
63+
## How to Reproduce
64+
65+
### Prerequisites
66+
67+
```bash
68+
# Install GitHub CLI (macOS)
69+
brew install gh
70+
71+
# Login to GitHub
72+
gh auth login
73+
74+
# Clone the repository
75+
git clone https://github.com/TheSuperHackers/GeneralsGameCode.git
76+
cd GeneralsGameCode
77+
78+
# Fetch all branches
79+
git fetch --all
80+
```
81+
82+
### Run the Analysis
83+
84+
```bash
85+
# Option 1: Use the provided script
86+
python3 scripts/analyze_pr_conflicts.py
87+
88+
# Option 2: Manual step-by-step
89+
# See "Manual Analysis" section below
90+
```
91+
92+
### Script Output
93+
94+
The script produces:
95+
1. **Console output**: Formatted report with statistics and details
96+
2. **JSON file**: `pr_conflict_analysis.json` with complete data for further processing
97+
98+
## Manual Analysis (Without Script)
99+
100+
If you want to manually check conflicts:
101+
102+
### 1. Get Open PRs
103+
104+
```bash
105+
gh pr list --limit 25 --json number,title,files --repo TheSuperHackers/GeneralsGameCode > prs.json
106+
```
107+
108+
### 2. Check Specific File Conflicts
109+
110+
```bash
111+
# See which PRs touch a specific file
112+
cat prs.json | jq '.[] | select(.files[].path | contains("Language.h")) | {number, title}'
113+
114+
# See all files in a specific PR
115+
gh pr view 1703 --json files --jq '.files[].path'
116+
```
117+
118+
### 3. Compare Your Branch
119+
120+
```bash
121+
# See what files your branch changes
122+
git diff --name-only main..bobtista/unify-common
123+
124+
# Check if a specific file conflicts with open PRs
125+
FILE="Generals/Code/GameEngine/Include/Common/Language.h"
126+
git diff --name-only main..bobtista/unify-common | grep "$FILE" && echo "File in your branch"
127+
gh pr list --json number,files --jq ".[] | select(.files[].path == \"$FILE\") | .number"
128+
```
129+
130+
## Understanding the Results
131+
132+
### Example Conflict
133+
134+
```
135+
Branch: bobtista/unify-common
136+
PR #1739: refactor(language.h): remove unused and redundant defines
137+
Conflicting files (1):
138+
- Generals/Code/GameEngine/Include/Common/Language.h
139+
```
140+
141+
**Interpretation:**
142+
- Your `unify-common` branch moves/modifies `Language.h`
143+
- PR #1739 also modifies `Language.h`
144+
- If both merge, there WILL be a conflict
145+
146+
**Resolution Options:**
147+
1. Merge PR #1739 first, then rebase your branch
148+
2. Exclude `Language.h` from your unification
149+
3. Coordinate with PR author to merge yours first, they rebase theirs
150+
151+
### CMakeLists.txt Conflicts
152+
153+
```
154+
PR #1594: feat(crashdump): Add crash dump functionality
155+
Conflicting files (1):
156+
- Core/GameEngine/CMakeLists.txt
157+
```
158+
159+
**Why low risk:**
160+
- CMakeLists.txt conflicts happen because both PRs add new files/dependencies
161+
- Resolution is simple: Keep both changes
162+
- No code logic conflicts
163+
164+
## Limitations
165+
166+
### What This Analysis Catches
167+
168+
✅ File-level conflicts (same file modified in both)
169+
✅ Moved/renamed files
170+
✅ Added/deleted files
171+
172+
### What This Analysis Misses
173+
174+
❌ Semantic conflicts (code logic that breaks across files)
175+
❌ Build dependency conflicts
176+
❌ Header include path changes
177+
❌ API/interface changes that affect other code
178+
179+
### False Positives
180+
181+
Some "conflicts" might not actually conflict:
182+
- Changes to different parts of the same file
183+
- Changes that are compatible/complementary
184+
- One PR adds code, other removes different code
185+
186+
**Always review the actual changes, not just file names!**
187+
188+
## Recommendations for Use
189+
190+
### Before Creating a Large Refactor/Unification PR
191+
192+
1. Run this analysis
193+
2. Identify high-risk conflicts
194+
3. Coordinate with those PR authors
195+
4. Consider excluding conflicting files temporarily
196+
197+
### After Running Analysis
198+
199+
1. **Post results** in your PR description
200+
2. **Tag affected authors** in comments
201+
3. **Propose a merge order** to maintainers
202+
4. **Offer to help** with rebasing
203+
204+
### For Maintainers
205+
206+
1. Use this to plan merge order
207+
2. Prioritize by:
208+
- Bug fixes (highest priority)
209+
- Features (merge or delay?)
210+
- Refactors (can usually wait)
211+
3. Batch similar PRs together
212+
213+
## Example Workflow
214+
215+
```bash
216+
# 1. Create your unification branch
217+
git checkout -b unify-my-feature
218+
219+
# 2. Before making changes, run analysis
220+
python3 scripts/analyze_pr_conflicts.py
221+
222+
# 3. Review conflicts in your area
223+
# - If few/no conflicts: Proceed
224+
# - If many conflicts: Coordinate with team
225+
226+
# 4. Make your changes, excluding problem files
227+
228+
# 5. Create PR with analysis in description
229+
230+
# 6. Update PR as other PRs merge
231+
git fetch --all
232+
git rebase origin/main
233+
python3 scripts/analyze_pr_conflicts.py # Re-run to see new state
234+
```
235+
236+
## Data Storage
237+
238+
The script saves detailed JSON output:
239+
240+
```json
241+
{
242+
"branches": {
243+
"bobtista/unify-common": ["file1.h", "file2.cpp", ...]
244+
},
245+
"conflicts": {
246+
"bobtista/unify-common": {
247+
"1739": {
248+
"title": "refactor(language.h): ...",
249+
"author": "Skyaero42",
250+
"files": ["Generals/Code/GameEngine/Include/Common/Language.h"]
251+
}
252+
}
253+
},
254+
"risk_categories": {
255+
"low": [...],
256+
"medium": [...],
257+
"high": [...]
258+
}
259+
}
260+
```
261+
262+
You can use this JSON for:
263+
- Custom analysis
264+
- Integration with CI/CD
265+
- Tracking over time
266+
- Automated notifications
267+
268+
## Questions?
269+
270+
If you have questions about the methodology:
271+
1. Read the script source: `scripts/analyze_pr_conflicts.py`
272+
2. Check the GitHub CLI docs: https://cli.github.com/manual/
273+
3. Ask in the PR discussion
274+
275+
## Credits
276+
277+
Analysis methodology developed by @bobtista for PR #1736.
278+
279+
280+

Core/GameEngine/CMakeLists.txt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ set(GAMEENGINE_SRC
146146
# Include/GameClient/ChallengeGenerals.h
147147
# Include/GameClient/ClientInstance.h
148148
Include/GameClient/ClientRandomValue.h
149-
Include/GameClient/Color.h
149+
#Include/GameClient/Color.h
150150
# Include/GameClient/CommandXlat.h
151151
# Include/GameClient/ControlBar.h
152152
Include/GameClient/ControlBarResizer.h
@@ -208,7 +208,7 @@ set(GAMEENGINE_SRC
208208
Include/GameClient/Module/AnimatedParticleSysBoneClientUpdate.h
209209
Include/GameClient/Module/BeaconClientUpdate.h
210210
Include/GameClient/Module/SwayClientUpdate.h
211-
Include/GameClient/Mouse.h
211+
#Include/GameClient/Mouse.h
212212
# Include/GameClient/ParabolicEase.h
213213
# Include/GameClient/ParticleSys.h
214214
# Include/GameClient/PlaceEventTranslator.h
@@ -687,7 +687,7 @@ set(GAMEENGINE_SRC
687687
# Source/Common/version.cpp
688688
Source/Common/WorkerProcess.cpp
689689
# Source/GameClient/ClientInstance.cpp
690-
Source/GameClient/Color.cpp
690+
#Source/GameClient/Color.cpp
691691
# Source/GameClient/Credits.cpp
692692
# Source/GameClient/Display.cpp
693693
# Source/GameClient/DisplayString.cpp

FILES_TO_REVERT.md

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
# Files to Revert from Unification PRs
2+
3+
This document lists files that need to be reverted from the unification PRs to avoid conflicts with active external PRs.
4+
5+
## Summary
6+
7+
**Total files to revert: 21**
8+
- From `bobtista/unify-common`: 12 files
9+
- From `bobtista/unify-gameclient`: 3 files
10+
- From `bobtista/unify-gamelogic`: 1 file
11+
- From `bobtista/unify-network`: 5 files
12+
13+
## Files to Revert
14+
15+
### From bobtista/unify-common (12 files)
16+
17+
**Conflicting PRs:** #654 (tomsons26), #670 (zzambers), #688 (tomsons26), #1066 (xezon), #1703 (Stubbjax), #1741 (Skyaero42)
18+
19+
1. Generals/Code/GameEngine/Include/Common/BitFlagsIO.h
20+
2. Generals/Code/GameEngine/Include/Common/DataChunk.h
21+
3. Generals/Code/GameEngine/Include/Common/GameType.h
22+
4. Generals/Code/GameEngine/Include/Common/List.h
23+
5. Generals/Code/GameEngine/Include/Common/Money.h
24+
6. Generals/Code/GameEngine/Include/Common/Radar.h
25+
7. Generals/Code/GameEngine/Include/Common/Recorder.h
26+
8. Generals/Code/GameEngine/Include/Common/SpecialPowerMaskType.h
27+
9. Generals/Code/GameEngine/Include/Common/StackDump.h
28+
10. Generals/Code/GameEngine/Source/Common/NameKeyGenerator.cpp
29+
11. Generals/Code/GameEngine/Source/Common/System/GameCommon.cpp
30+
12. Generals/Code/GameEngine/Source/Common/System/StackDump.cpp
31+
32+
### From bobtista/unify-gameclient (3 files)
33+
34+
**Conflicting PRs:** #654 (tomsons26), #688 (tomsons26)
35+
36+
1. Generals/Code/GameEngine/Include/GameClient/Color.h
37+
2. Generals/Code/GameEngine/Include/GameClient/Mouse.h
38+
3. Generals/Code/GameEngine/Source/GameClient/Color.cpp
39+
40+
### From bobtista/unify-gamelogic (1 file)
41+
42+
**Conflicting PRs:** #654 (tomsons26)
43+
44+
1. Generals/Code/GameEngine/Include/GameLogic/Module/ConvertToCarBombCrateCollide.h
45+
46+
### From bobtista/unify-network (5 files)
47+
48+
**Conflicting PRs:** #688 (tomsons26), #1049 (Caball009), #1668 (jbremer)
49+
50+
1. Generals/Code/GameEngine/Include/GameNetwork/ConnectionManager.h
51+
2. Generals/Code/GameEngine/Include/GameNetwork/GameSpy/StagingRoomGameInfo.h
52+
3. Generals/Code/GameEngine/Include/GameNetwork/LANGameInfo.h
53+
4. Generals/Code/GameEngine/Include/GameNetwork/networkutil.h
54+
5. Generals/Code/GameEngine/Source/GameNetwork/NetPacket.cpp
55+
56+
## Notes
57+
58+
- All CMakeLists.txt conflicts are acceptable and easy to resolve
59+
- All conflicts with your own PRs (#1675, #1680, #1682, #1683, #1733, #1734, #1736, #1737, #1738) can be managed as they're yours
60+
- slurmlord's PR #1594 only conflicts with CMakeLists.txt, which is fine
61+
62+
## Next Steps
63+
64+
1. Checkout each unification branch
65+
2. Revert the moves for these 3 files
66+
3. Update CMakeLists.txt to not include these files in Core
67+
4. Force push the updated branches
68+

0 commit comments

Comments
 (0)