unify(thingtemplate): Merge and move ThingTemplate to Core - #3324
Conversation
|
Understand this PR’s impact Explore downstream dependencies and potential security impact with Blast Radius. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. WalkthroughThe change enables the Core ChangesThingTemplate platform integration
Priority: ⬇️ Low Estimated code review effort: 4 (Complex) | ~45 minutes Change: Refactor 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
xezon
left a comment
There was a problem hiding this comment.
There are a lot more things in ThingTemplate that can be merged. Maybe merge the whole thing?
i do agree, but it will be huge unrelated change as i need those specific changes for the world builder to get merged, but i will see what i can do |
|
I looked over ThingTemplate and it is relevatively low level. So it seems possible to merge the whole thing. |
yes on it, will update the pr soon. |
1df0d09 to
d182473
Compare
d182473 to
02db325
Compare
xezon
left a comment
There was a problem hiding this comment.
This change has a number of RTS_GENERALS and RTS_ZEROHOUR defines. Are all of these justified?
i try to keep both games as pure as possible, without adding things from zerohour to gen or vice versa, is it possible to port some of guarded things to gen, if needed. |
02db325 to
4040697
Compare
|
Some guidelines for merging:
It is the job of a merge to merge as much as is possible while not violating the above. |
4040697 to
f3a482f
Compare
as for now i removed the guard around OverrideableByLikeKind registration. it parsing is now shared while gen keeps its original module inheritance whenever CRC or save compatibility is enabled, also it adopts the Zero Hour inheritance rules when both flags are disabled ready for re review |
f3a482f to
9483c40
Compare
I assume you are right, will improve it based on the feedback. |
9483c40 to
df0fbfd
Compare
df0fbfd to
e9a0463
Compare
xezon
left a comment
There was a problem hiding this comment.
Logically looks very good now. I did not see mistake.
@Caball009 @Skyaero42 Can one of you run this against a number of Generals Replays?
Will do |
e9a0463 to
3e4bf9b
Compare
Skyaero42
left a comment
There was a problem hiding this comment.
Went through about 100 replays, no issues found.
|
Rename the first commit to |
3e4bf9b to
620adbd
Compare
Done, it now renamed |
|
Thank you. |
Merge with Rebase
Merges the Generals and Zero Hour ThingTemplate implementations using Zero Hour as the baseline, then moves the shared header and source to Core in preparation for the WorldBuilder merge.
Validation:
"i used some AI assistance in analysis and validation"