Page MenuHomeWildfire Games

Remove duplication of code from WaterManager
AcceptedPublic

Authored by Angen on Sun, Nov 10, 2:01 PM.

Details

Reviewers
vladislavbelov
Summary

Use already defined function in LoadWaterTextures with the same code.
Fixes the TODO from rP16388 / rP16389

Test Plan

Check replaced code is identical to one inside function and it works

Event Timeline

Angen created this revision.Sun, Nov 10, 2:01 PM
Owners added a subscriber: Restricted Owners Package.Sun, Nov 10, 2:02 PM

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

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/556/display/redirect

elexis edited the summary of this revision. (Show Details)Sun, Nov 10, 2:11 PM

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

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1070/display/redirect

vladislavbelov accepted this revision.Sun, Nov 10, 3:08 PM
vladislavbelov added inline comments.
source/renderer/WaterManager.cpp
209

Only disadvantage that I see, you don't reuse pathname. But it shouldn't touch performance.

379

Could you also replace (int)i+1 by static_cast<int>(i) + 1 or replace format %02d by %02zu?

This revision is now accepted and ready to land.Sun, Nov 10, 3:08 PM
Angen updated this revision to Diff 10298.Sun, Nov 10, 3:23 PM

static cast

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

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/558/display/redirect

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

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1072/display/redirect

Stan added a subscriber: Stan.Mon, Nov 11, 10:44 AM

Anyway we can test it actually works on GLES ?

Stan added inline comments.Wed, Nov 13, 10:24 PM
source/renderer/WaterManager.cpp
377

Any reason not to use int here ?

379

Can't we replace ARRAY_SIZE(pathname) by PATH_MAX so we don't call a macro to do whatever each time ?

Angen added inline comments.Mon, Dec 2, 2:06 PM
source/renderer/WaterManager.cpp
379

It will be done in another patch as it is case in more places and kind of unrelated to reason of this patch.

Stan added inline comments.Mon, Dec 2, 2:23 PM
source/renderer/WaterManager.cpp
372–373

Doxygen.

379

Sure, maybe it's just an optimization. I don't know if that macro call is cached...