JPS - make the JPC cache usable again (rP22225 fix)
ClosedPublic
Actions

Authored by wraitii on May 11 2019, 2:38 PM.

Details

Reviewers

Itms

Group Reviewers

Restricted Owners Package

(Owns No Changed Paths)

Commits

rP22301: JPS - make the JPC cache usable again (rP22225 fix)

Summary

rP22225 hurriedly fixed a crash, but made the JPC cache fail - though we are not using it and it might fail for other reasons still.

Test Plan

Review code

Diff Detail

Repository

rP 0 A.D. Public Repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

wraitii created this revision.May 11 2019, 2:38 PM

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

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

Harbormaster completed remote builds in B7435: Diff 7970.May 11 2019, 3:02 PM

Stan added a subscriber: Stan.May 11 2019, 5:30 PM

Stan added inline comments.

source/simulation2/helpers/LongPathfinder.cpp
721 ↗	(On Diff #7970)	Use the explicit type, see comment on the commit :)

wraitii retitled this revision from JPS - make the JPC cache usable again (rP22248 fix) to JPS - make the JPC cache usable again (rP22225 fix).May 25 2019, 9:31 AM

wraitii edited the summary of this revision. (Show Details)

What Stan said, unless the complete type is actually very long (not on desktop, I can't check).

I liked the extra line between lines 722 and 723 but at that point I'm just being annoying ?

Apart from that this is good for me.

no more auto (tbh here I feel like it would have been legitimate, but I'm definitely more on the "auto" side than most of you guys).

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

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

Harbormaster completed remote builds in B7579: Diff 8117.May 25 2019, 3:13 PM

Itms added inline comments.May 25 2019, 4:41 PM

source/simulation2/helpers/LongPathfinder.cpp
722 ↗	(On Diff #8117)	Add a space between the two closing `>`. Sub-par compilers (can't remember which ones) sometimes mistake that for a `>>` operator.

Stan added inline comments.May 25 2019, 4:43 PM

source/simulation2/helpers/LongPathfinder.cpp
722 ↗	(On Diff #8117)	That's... Retarded...

wraitii added inline comments.May 25 2019, 4:44 PM

source/simulation2/helpers/LongPathfinder.cpp
722 ↗	(On Diff #8117)	(2019 and this is still an issue -_-) Are these compilers actually supported nowadays? Does VS2013 exhibit this?

Itms added inline comments.May 25 2019, 5:04 PM

source/simulation2/helpers/LongPathfinder.cpp
722 ↗	(On Diff #8117)	I actually think splitting them helps with readability, so I'd push for doing it anyway. But yes IIRC the issue still exists with 2013 and is fixed in 2015.