Current goal of the series of patches is to make CModel and SDecal storing immutable CMaterial to be in one-to-one relationship with art/materials (later we might introduce CMaterialInstance for per-object settings). The next step would be preparing for removing and removing AddFlagsRec and RemoveShadowsRec/RemoveShadows as they change the material.
Details
Details
- Reviewers
- None
- Group Reviewers
Restricted Owners Package (Owns No Changed Paths) - Commits
- rP27841: Refactors models and materials, part 1, reduces amount of mutable properties.
- Apply the patch and compile the game
- Check that the game looks the same
Diff Detail
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
| source/graphics/Model.cpp | ||
|---|---|---|
| 41 ↗ | (On Diff #22166) | prefer {}, as per https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Res-list |
| 66 ↗ | (On Diff #22166) | m_Props.clear(); Doesn't has to be done manually. |
| source/graphics/Model.h | ||
| 44 ↗ | (On Diff #22166) | What do you mean by "world information"? |
| 233 ↗ | (On Diff #22166) | I would use direct initialization (with braces) instead of copy ititialization. |
| 237 ↗ | (On Diff #22166) | This results in const std::shared_ptr<CModelDef> while it should be const std::shared_ptr<const CModelDef> i think. |
| source/graphics/ObjectEntry.cpp | ||
| 136–155 ↗ | (On Diff #22166) | Could you extract this part into an own function. |
Comment Actions
Build failure - The Moirai have given mortals hearts that can endure.
Link to build: https://jenkins.wildfiregames.com/job/macos-differential/7272/display/redirect
Comment Actions
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/8361/display/redirect
| source/graphics/Model.h | ||
|---|---|---|
| 237 ↗ | (On Diff #22166) | You don't have to use an alias. |
| source/graphics/Model.h | ||
|---|---|---|
| 237 ↗ | (On Diff #22166) | It'd be consistent with other code. We use only CModelDefPtr, but I don't mind to change that in the future. |