Page MenuHomeWildfire Games

Allow limiting the max number of corpses simultaneously visible in the game
AbandonedPublic

Authored by Stan on Aug 5 2020, 1:04 PM.

Details

Summary

Currently having a lot of corpses lying around can hit a toll on low end computers due to the high number of drawcalls. This patch allows player to mitigate it by reducing the max number of corpses

Test Plan

Run a game and try different options.

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
wraitii accepted this revision.Aug 6 2020, 9:04 AM

This looks fine now :)

binaries/data/mods/public/simulation/components/tests/test_Health.js
166

There's a _reset function, might be better to use that and check against 0.

This revision is now accepted and ready to land.Aug 6 2020, 9:04 AM
wraitii requested changes to this revision.Aug 6 2020, 9:51 AM

Following discussion on IRC -> it seems we should instead delete the oldest corpse, as this looks rather ugly.

I suggest simply storing entity_id_t in an std::deque, and you can pop_back -> DeleteEntity or whatever it's called.

This revision now requires changes to proceed.Aug 6 2020, 9:51 AM
Stan updated this revision to Diff 13097.Aug 6 2020, 11:06 AM

Third approach more like @nani's autociv

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

Linter detected issues:
Executing section Source...

Project wide:
**** CPPCheckBear (syntaxError) [Section: Source | Severity: MAJOR] ****
!    ! Code 'classICmpUnitRenderer:' is invalid C code. Use --std or --language to configure the language.
Executing section JS...
**** ESLintBear (no-multi-spaces) [Section <empty> | Severity NORMAL] ****
!    ! Multiple spaces found before 'numberOfCorpses'.
[----] /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Health.js
[++++] /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Health.js
[  85] 	"CanAddCorpse": () => { return  numberOfCorpses < 1 },
[  85] 	"CanAddCorpse": () => { return numberOfCorpses < 1 },
**** ESLintBear (semi) [Section <empty> | Severity NORMAL] ****
!    ! Missing semicolon.
[----] /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Health.js
[++++] /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Health.js
[  85] 	"CanAddCorpse": () => { return  numberOfCorpses < 1 },
[  85] 	"CanAddCorpse": () => { return  numberOfCorpses < 1; },

binaries/data/mods/public/simulation/components/tests/test_Health.js
[  85] »   "CanAddCorpse":·()·=>·{·return··numberOfCorpses·<·1·},
**** JSHintBear [Section: JS | Severity: NORMAL] ****
!    ! Missing semicolon.
Executing section cli...

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

wraitii accepted this revision.Aug 6 2020, 12:27 PM

Still working, overall still looks fine to me. I agree that it's better that way.

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

Woudl'nt a slider be better?

601

Might be worth adding a max here. I don't think the value makes much sense above say 512 corpses, and it would start possibly being problematic with a very large std::deque.

binaries/data/mods/public/simulation/components/tests/test_Health.js
85
109–110
source/simulation2/components/CCmpUnitRenderer.cpp
124

seems a bit weird to add it here between those two frame-related variables. I'd plop it above, with a little comment.

196

Can't use size_t directly here?

220

I don't think you need that -> you're calling RemoveCorpse in Deinit() which means the entity is already destroying itself.

This revision is now accepted and ready to land.Aug 6 2020, 12:27 PM
Stan updated this revision to Diff 13102.Aug 6 2020, 1:02 PM
Stan marked 11 inline comments as done.

Inlines, linter

Stan marked an inline comment as done.Aug 6 2020, 1:04 PM
Stan added inline comments.
source/simulation2/components/CCmpUnitRenderer.cpp
196

Could but then I'd have to cast the 0 for fun :D

Vulcan added a comment.Aug 6 2020, 1:06 PM

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

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

Stan retitled this revision from Allow limiting the number of ragdolls in the game to Allow limiting the max number of corpses simultaneously visible in the game.Aug 6 2020, 1:31 PM

I haven't checked the code, but it looks waaay better in-game :)

wraitii added inline comments.Aug 6 2020, 1:56 PM
source/simulation2/components/CCmpUnitRenderer.cpp
219

This reminds me -> You should swap and pop here instead.

Nescio added a subscriber: Nescio.Aug 6 2020, 1:58 PM

Does this only affect corpses, or also other things?
I don't mind seeing blood per se, but currently blood stays visible much longer than corpses, which is both unrealistic and slightly annoying: I don't really care where units were killed long ago.

Stan added a comment.Aug 6 2020, 2:02 PM

Everything that decays. So structures, ships, units etc.

Only removing blood is more complicated, this patch goal is just to limit the number of decaying things, not to remove blood

Nescio added a comment.Aug 6 2020, 2:15 PM

Currently (I didn't try your patch) rubble disappears after a minute or so, a lot quicker than corpses, and blood remains visible much longer. I think it would be better they all disappear at the same speed, and oldest entities go first.
If your patch does that, great!

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

How about adding a sensible default? Perhaps 100?

Stan added a comment.Aug 6 2020, 2:24 PM

The patch does the following

Set up a max number.

Each time an object creates a corpse, increment the current number,

Each time a corpse disappear decrement the current number,

If a new corpse is requested while there are none available (current number = max number) delete the oldest corpse we have and add the new one.

Stan updated this revision to Diff 13104.Aug 6 2020, 2:30 PM

Iterator mumbo jumbo

Nescio added a comment.Aug 6 2020, 2:33 PM

That sounds good and sensible.
As for corpses, does that include rubble and blood, or are those different things?

Vulcan added a comment.Aug 6 2020, 2:36 PM

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

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

wraitii added inline comments.Aug 6 2020, 2:42 PM
source/simulation2/components/CCmpUnitRenderer.cpp
220

pop_

OptimusShepard added a subscriber: OptimusShepard.
OptimusShepard added inline comments.
binaries/data/config/default.cfg
64

Maybe 50, or 100 by default?

Stan updated this revision to Diff 13106.Aug 6 2020, 2:56 PM

Fix typo,

Corpses include rubble indeed basically anything with decay set to active, if your limit is 5 and the first in the list was the rubble, then it will disappear to allow for one more corpse.

Blood is defined in actors, so if the actors disappear, the blood disappears. The reason you are actually seeing blood much longer, is that it works as a decal, and so if you see it that means the whole unit is still there below the ground

Stan added a comment.EditedAug 6 2020, 2:57 PM
This comment has been deleted.
binaries/data/config/default.cfg
64

Why limit it ?

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

Default is -1 (Unlimited)

Vulcan added a comment.Aug 6 2020, 3:00 PM

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

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

Angen added a subscriber: Angen.Aug 6 2020, 3:07 PM
Angen added inline comments.
source/simulation2/components/CCmpUnitRenderer.cpp
219

I have feeling something went wrong here.
You are looking for entity and if you find it, you do nothing with that information.
Also L226 and L227 are doing nothing.

OptimusShepard added inline comments.Aug 6 2020, 3:09 PM
binaries/data/config/default.cfg
64

I guess most people wont notice negative optical effects, when the setting is enabled. And I also guess, most people have no idea, that this setting could really improve the performance in late game. So setting this option by default would be an improvement, I think.

Angen added inline comments.Aug 6 2020, 3:14 PM
source/simulation2/components/CCmpUnitRenderer.cpp
219

ok, now i got it :D

wraitii added inline comments.Aug 6 2020, 3:17 PM
binaries/data/config/default.cfg
64

Hm, just to be clear -> After a while, decaying components such as corpses do disappear already.

vladislavbelov added inline comments.Aug 7 2020, 9:08 PM
binaries/data/mods/public/gui/options/options.json
602

Why 600?

binaries/data/mods/public/simulation/components/Health.js
328

That means after option change player won't see units destroyed before the change.

source/simulation2/components/CCmpUnitRenderer.cpp
41

Wrong order.

215

It does linear search, so in case of deleting many corpses per turn it might be expensive.

source/simulation2/components/ICmpUnitRenderer.cpp
26

DEFINE_INTERFACE_METHOD_CONST_0.

source/simulation2/components/ICmpUnitRenderer.h
54

virtual bool CanAddCorpse() const = 0;.

Stan updated this revision to Diff 13141.Aug 7 2020, 9:25 PM
Stan marked 13 inline comments as done.

Fix inlines

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

I didn't put any at first @wraitii asked me to add something so I put 600 if you want more you can always use -1

602

Also because deque might be slow for big sizes.

binaries/data/mods/public/simulation/components/Health.js
328

We don't support graphics hotloading

source/simulation2/components/CCmpUnitRenderer.cpp
215

What do you suggest then?

Vulcan added a comment.Aug 7 2020, 9:27 PM

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

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

Vulcan added a comment.Aug 7 2020, 9:28 PM

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

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

Vulcan added a comment.Aug 7 2020, 9:28 PM

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

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

wraitii added inline comments.Aug 7 2020, 11:32 PM
source/simulation2/components/CCmpUnitRenderer.cpp
215

This is partly the reason why I suggested keeping a small 'max' number. I think for < 500 it's going to be generally irrelevant. I might be wrong on that though - perhaps a number as low as 50 is good enough.

If you keep the list sorted (precludes doing the pop-swap trick), you can use a binary search.

wraitii added inline comments.Aug 8 2020, 8:28 AM
source/simulation2/components/CCmpUnitRenderer.cpp
215

BTW you'll have to do some o(N) thing somewhere -> either search can be amortised with binary search, but then you can pop-swap so erasing will be o(N/2), or you keep like this, and searching is o(N) but erasing o(1).

It'd need to be tested to know which is faster and whether any of that is a real problem, which I doubt.

Nescio removed a subscriber: Nescio.Aug 8 2020, 9:04 AM
Stan updated this revision to Diff 13147.Aug 8 2020, 5:08 PM

Make it black and white. Apparently storing into a dequeue has a too big performance impact.

Stan requested review of this revision.Aug 8 2020, 5:10 PM

@vladislavbelov do your thang.

Vulcan added a comment.Aug 8 2020, 5:29 PM

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

Linter detected issues:
Executing section Source...
Executing section JS...
Executing section cli...

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

A dying animation is not possible here? Or at least a simple visual plop effect?

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

You should also write that a restart is needed.

Stan added a comment.Aug 8 2020, 6:20 PM

A dying animation is not possible here? Or at least a simple visual plop effect?

Dying animation is the corpse ^^

In D2936#128281, @Stan wrote:

Dying animation is the corpse ^^

Ok. Is it possible, to splitt the corpse animation into dying and decay animations, and than remove only the decay animation? I guess, only the long decay animation is performance critical?

Stan added a comment.Aug 8 2020, 6:37 PM
In D2936#128281, @Stan wrote:

Dying animation is the corpse ^^

Ok. Is it possible, to splitt the corpse animation into dying and decay animations, and than remove only the decay animation? I guess, only the long decay animation is performance critical?

Not that I know of.

Stan abandoned this revision.Aug 17 2020, 10:45 AM

Not worth it anymore.

In D2936#129095, @Stan wrote:

Not worth it anymore.

What happened to this diff? Will there be some better alternative?

Stan added a comment.Aug 19 2020, 6:58 PM

Well most of the functionnality was removed on vlad's feedback, and I'm not happy about it wraitii also said it was not worth it in that case, so i'd rather not work on something that doesn't make me want to commit it. And so no nothing is planned.