Page MenuHomeWildfire Games

Fixes includes and forward declarations in CPatchRData
ClosedPublic

Authored by vladislavbelov on Mon, Jun 3, 10:30 PM.

Details

Reviewers
wraitii
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP22371: Fixes includes and forward declarations in CPatchRData.
Summary

There were missed includes and forward declarations. But it was working because the CPatchRData.h was luckily included in a good place.

Test Plan
  1. Apply the patch and compile the game
  2. Make sure that it still compiles

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

vladislavbelov created this revision.Mon, Jun 3, 10:30 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/differential/1597/display/redirect

wraitii requested changes to this revision.Wed, Jun 5, 9:32 PM
wraitii added a subscriber: wraitii.
wraitii added inline comments.
source/renderer/PatchRData.h
37 ↗(On Diff #8301)

This doesn't compile. http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2008/n2764.pdf

Need to add : short to this and the definition for me.

This revision now requires changes to proceed.Wed, Jun 5, 9:32 PM
vladislavbelov added inline comments.Wed, Jun 5, 9:40 PM
source/renderer/PatchRData.h
37 ↗(On Diff #8301)

I'd prefer to add include, because the storing type can be changed.

wraitii added inline comments.Wed, Jun 5, 9:42 PM
source/renderer/PatchRData.h
37 ↗(On Diff #8301)

Sure.

Fixes compilation.

vladislavbelov marked 2 inline comments as done.Wed, Jun 5, 9:50 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/1618/display/redirect

wraitii accepted this revision.Thu, Jun 6, 9:32 PM

Compiles now indeed. Good cleanup, cheers.

This revision is now accepted and ready to land.Thu, Jun 6, 9:32 PM
Stan added a subscriber: Stan.Thu, Jun 6, 9:55 PM
Stan added inline comments.
source/renderer/PatchRData.h
31 ↗(On Diff #8328)

Won't that lead to redefinition ?

vladislavbelov added inline comments.Thu, Jun 6, 9:57 PM
source/renderer/PatchRData.h
31 ↗(On Diff #8328)

Nope, it's declaration not definition.

Stan added inline comments.Thu, Jun 6, 9:58 PM
source/renderer/PatchRData.h
31 ↗(On Diff #8328)

Yeah but it's already declared in the file you included so the forward declaration doesn't make sense, does it ?

vladislavbelov added inline comments.Thu, Jun 6, 10:08 PM
source/renderer/PatchRData.h
31 ↗(On Diff #8328)

Yes, it makes sense in terms of the code support. You don't need to fix the code if you remove the only include.

This revision was automatically updated to reflect the committed changes.