refactor(pathfinding): simplify functions to improve readability - Part 1

[Mauller]

Rebase and merge

This PR cleans up various functions within the pathfinding, the goal to improve readability where possible and to improve performance where possible.

I have broken each function down into it's own commit to aid review.

This PR covers the functions

• PathfindCell::releaseInfo
• Pathfinder::checkChangeLayers
• Pathfinder::examineNeighboringCells
• Pathfinder::internal_findHierarchicalPath

A few of the refactors cover code simplification by inverting the logic and returning early or by simplifying functionality.

---

Todo

• Replicate in generals
• Test performance improvements, if any.

[xezon]

Generally, I do not think you need to merge multiline conditions into single line condition if it takes away the opportunity to place breakpoints.

[Mauller]

Generally, I do not think you need to merge multiline conditions into single line condition if it takes away the opportunity to place breakpoints.

It's the only real way to gain performance right now. if it needs debugging then the conditions can always be broken out again and checked at the time.

[xezon]

It's the only real way to gain performance right now. if it needs debugging then the conditions can always be broken out again and checked at the time.

Are you sure it gives performance? I expect that the generated assembler will be the same.

[xezon]

Here is a simple example with godbolt:

On O1 the generated code is already the same.

https://godbolt.org/z/7q7qcKGq8

[Mauller]

Here is a simple example with godbolt:

On O1 the generated code is already the same.

https://godbolt.org/z/7q7qcKGq8

The main thing would be if VC6 does this or something different.

I am certainly getting better performance from some of the changes.

[xezon]

It could be placebo. In the hope of better performance it is perceived as such. Merging conditions is not a strategy that is expected to give performance gains. At least I am not aware of it. Assembler has no concept of merged conditionals either. It separates merged conditions into separate instructions.

[Mauller]

It could be placebo. In the hope of better performance it is perceived as such. Merging conditions is not a strategy that is expected to give performance gains. At least I am not aware of it. Assembler has no concept of merged conditionals either. It separates merged conditions into separate instructions.

I mean this PR in general has visible performance improvements when i run it compared to without the changes.
But that could be due to some of the other tweaks simplifying code in places and early returning.

I will take a look later and see if not combining the conditionals makes any difference.
It could just be the other structural and functional changes that i made which are giving the boost.

[Mauller]

Tweaked and pushed

[xezon]

Is this ready to continue review?

[Mauller]

Is this ready to continue review?

just need to make those two tweaks, just a sec.

[Mauller]

Made the tweaks, should be good now

[xezon]

Is the change title still accurate?

[Mauller]

Addressed comments and updated

[Mauller]

will sort out replicating to generals in a bit

[Mauller]

copied across to generals, testing each change at a time, hence the split commits. No real total performance improvement but i think it helps some areas that lag at times.

Generals has a small difference in examineNeighbouringCells which was causing mismatching and some annoyance. But it was a single extra value within:

if (info.allyFixedCount>0) {
				newCostSoFar += 3*COST_DIAGONAL*info.allyFixedCount;

while zero hour ommits the info.allyFixedCount portion

Not sure how we want to merge these, as individual commits or as squashed?
Individual would make it easier to test which change may have caused a mismatch, but with all my testing i did not see any mismatching.

If we go with individual commits i can rename all the commits to something more suitable etc.

[xezon]

Can do 4 commits. Merge the Generals replications into the respective ZH commits.

For commit title, use "refactor(pathfinder)"

Also, sentence behind colon always starts with upper case.

And append Pull number to commit titles.

[Skyaero42]

ok

[Mauller]

Can do 4 commits. Merge the Generals replications into the respective ZH commits.

For commit title, use "refactor(pathfinder)"

Also, sentence behind colon always starts with upper case.

And append Pull number to commit titles.

Will look at doing that later, i will also pull out the function for checking cell extents into it's own commit as it will get used in other commits so don't want to lose it by mistake if the commit it is part of got reverted.

[Mauller]

Got the commits in order, should be good to go now, once this is merged i will tweak part 2 and add the changes to generals.