Page MenuHomeWildfire Games

optionally unrandomize entity appearance
AcceptedPublic

Authored by bb on Oct 13 2020, 10:43 PM.

Details

Reviewers
vladislavbelov
Freagarach
Trac Tickets
#5831
Summary

As claimed by @vladislavbelov on several occasions (e.g. https://wildfiregames.com/forum/topic/29654-request-sacrificial-knife/?tab=comments#comment-405553), having many different actors affects performance. A way to reduce the effect, but still allow many variants is to optionally disable the randomness. This should limit the number of different variants if disabled, but keeps the old performance if enabled.

Test Plan

Check that it actually improves performance
Check no OOS occurs
Comment on D3034
Think of better descriptions

Event Timeline

bb created this revision.Oct 13 2020, 10:43 PM

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

builderr-release-macos.txt
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libgraphics.a(precompiled.o) has no symbols

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1648/display/redirect

bb edited the summary of this revision. (Show Details)Oct 13 2020, 10:49 PM
bb edited the test plan for this revision. (Show Details)
bb requested review of this revision.Oct 13 2020, 10:51 PM
Freagarach added inline comments.
binaries/data/config/default.cfg
84

I guess you mean variants instead of graphics items?

85

graphics.entityvariety?

binaries/data/mods/public/gui/options/options.json
202

slight and boost are kind of opposite ;)

source/graphics/ObjectBase.cpp
568–569

This sentence is a continuation of Choose a random number in the interval [0..totalFreq) so with the full-stop there this needs to be changed as well.

569

I'm not sure what this all does, but can't you skip the whole searching for a match?

bb updated this revision to Diff 13640.Oct 16 2020, 8:47 PM
bb marked 4 inline comments as done.
bb added inline comments.
source/graphics/ObjectBase.cpp
569

We sometimes want to force a unit to take a certain form (choose some animation say), I think...

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

builderr-release-macos.txt
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libgraphics.a(precompiled.o) has no symbols

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1656/display/redirect

Freagarach added inline comments.Oct 19 2020, 10:24 AM
binaries/data/config/default.cfg
84

What is meant with remaining here?

binaries/data/mods/public/gui/options/options.json
202

small and boost are still. I suggest using improvement over boost. Then either slight or small as you wish,

source/graphics/ObjectBase.cpp
571

If randNum is 0 due to entityVariety == false, this will become negative immidiately, I think? (Assuming (*grp)[i].m_Frequency) is a positive integer.) Causing us to break at after the first loop with match == 0?
So it seems to me that we can just set match to 0 and skip the whole randNum thing?

vladislavbelov added inline comments.Oct 26 2020, 9:55 PM
binaries/data/mods/public/gui/options/options.json
202

Maybe Works best in a new match.? The "game" word (in context of match) in options seems a bit misleading to me.

source/graphics/ObjectBase.cpp
568

entityVariety might be not initialized afaik.

569

Are sure that you need to ask options for each group and each model?

bb updated this revision to Diff 13664.Oct 26 2020, 10:40 PM
Owners added subscribers: Restricted Owners Package, Restricted Owners Package.Oct 26 2020, 10:40 PM
bb added inline comments.Oct 26 2020, 10:42 PM
binaries/data/mods/public/gui/options/options.json
202

by the changes it now requires to restart a match, this doesn't have to be done like this, by adding some function setting the parameter on the fly

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

builderr-release-macos.txt
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libsimulation2.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libengine.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libgraphics.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libatlas.a(precompiled.o) has no symbols

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1675/display/redirect

vladislavbelov added inline comments.Oct 27 2020, 7:37 PM
binaries/data/mods/public/gui/options/options.json
203

rentityvariety > entityvariety.

source/graphics/ObjectBase.cpp
534

Btw, do we need to have the initial selection processing if we disable variety?

552

grp->front().m_VariantName.

Freagarach added inline comments.Oct 27 2020, 7:47 PM
source/graphics/ObjectBase.cpp
554

Can't you break here?

bb added inline comments.Oct 28 2020, 10:31 PM
source/graphics/ObjectBase.cpp
534

If the initital selections already selected something, we should honor that selection and not choose anything else.

554

*shouldn't

bb updated this revision to Diff 13672.Oct 28 2020, 10:34 PM

comments

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

builderr-release-macos.txt
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libsimulation2.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libgraphics.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libatlas.a(precompiled.o) has no symbols

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1682/display/redirect

Freagarach accepted this revision.Oct 29 2020, 8:05 AM

Tested, worked as advertised. It looks kinda strange to have all trees look exactly the same, but that is the point of this patch ;)
Textuals look good.

binaries/data/config/default.cfg
84

+.?

source/graphics/ObjectBase.cpp
554

Yeah,,, ^^

567

(You could revert the comment change now.)

This revision is now accepted and ready to land.Oct 29 2020, 8:05 AM

Tested, worked as advertised. It looks kinda strange to have all trees look exactly the same, but that is the point of this patch ;)

Have you compared performance?

Yes, I see a 10% improvement on combat demo (huge) (FPS increase, ms/frame and ms/turn decrease).

Stan updated the Trac tickets for this revision.Jan 1 2021, 6:16 PM

I think this should be put next to renderactors, and possibly have that as the lowest setting?