Page MenuHomeWildfire Games

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

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
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; 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

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)
Itms added a subscriber: Itms.May 25 2019, 10:10 AM

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.

wraitii updated this revision to Diff 8117.May 25 2019, 3:08 PM

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

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.

wraitii updated this revision to Diff 8128.May 25 2019, 5:11 PM

>> >>> > >

Itms accepted this revision.May 25 2019, 5:21 PM

Wait for the build, but yes from me.

This revision is now accepted and ready to land.May 25 2019, 5:21 PM

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

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

This revision was automatically updated to reflect the committed changes.