Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix windows path unresolving #14868

Merged
merged 2 commits into from Mar 11, 2018

Conversation

Projects
None yet
4 participants
@pchote
Copy link
Member

pchote commented Mar 1, 2018

This should in principle fix #11599, but I do not have a suitable system to test with.
Ping @GraionDilach.

@pchote pchote added this to the Next release milestone Mar 1, 2018

@pchote

This comment has been minimized.

Copy link
Member Author

pchote commented Mar 1, 2018

IMO this is probably too risky to take directly into a hotfix release without a proper playtest. If others agree then it and #11499 can move to Next + 1.

@GraionDilach

This comment has been minimized.

Copy link
Contributor

GraionDilach commented Mar 1, 2018

Will test it during the weekend then.

@reaperrr

This comment has been minimized.

Copy link
Contributor

reaperrr commented Mar 4, 2018

This actually breaks campaign sorting for good on Windows. No idea why, though.

Edit: Which supports

IMO this is probably too risky to take directly into a hotfix release without a proper playtest. If others agree then it and #11499 can move to Next + 1.

IMO, too.

@reaperrr reaperrr modified the milestones: Next release, Next + 1 Mar 7, 2018

@RoosterDragon

This comment has been minimized.

Copy link
Member

RoosterDragon commented Mar 9, 2018

This PR plus the following works for me:

var previews = modData.MapCache
	.Where(p => p.Status == MapStatus.Available)
	.Select(p => new
	{
		Preview = p,
		Index = missionMapPaths.IndexOf(Platform.UnresolvePath(p.Package.Name).Replace('\\', '/'))
	})
	.Where(x => x.Index != -1)
	.OrderBy(x => x.Index)
	.Select(x => x.Preview);

On Windows, the unresolved path uses \ as the path separator, this needs to be changed to / to match the config.

The ordering function also needs the same change - or you can rejig as above to avoid the duplication.

@pchote

This comment has been minimized.

Copy link
Member Author

pchote commented Mar 9, 2018

On Windows, the unresolved path uses \ as the path separator, this needs to be changed to / to match the config.

We should move this into UnresolvePath

@pchote pchote force-pushed the pchote:fix-windows-unresolve-path branch from 643937b to d35ec47 Mar 10, 2018

@pchote

This comment has been minimized.

Copy link
Member Author

pchote commented Mar 10, 2018

Updated. Can you please test again @RoosterDragon?


var previews = modData.MapCache
.Where(p => p.Status == MapStatus.Available && missionMapPaths.Contains(p.Package.Name))
.Where(p => p.Status == MapStatus.Available && missionMapPaths.Contains(Platform.UnresolvePath(p.Package.Name)))
.OrderBy(p => missionMapPaths.IndexOf(p.Package.Name));

This comment has been minimized.

@RoosterDragon

RoosterDragon Mar 10, 2018

Member

Need to UnresolvePath here too or it won't order correctly.

@RoosterDragon

This comment has been minimized.

Copy link
Member

RoosterDragon commented Mar 10, 2018

Works - however, do we also need to do the opposite for ResolvePath? i.e. Change '/' into the platform specific directory char?

@pchote

This comment has been minimized.

Copy link
Member Author

pchote commented Mar 10, 2018

Shouldn't do - Windows happily consumes / for paths.

@pchote pchote force-pushed the pchote:fix-windows-unresolve-path branch from d35ec47 to b24fcbb Mar 10, 2018

@pchote

This comment has been minimized.

Copy link
Member Author

pchote commented Mar 10, 2018

Updated.

@reaperrr

This comment has been minimized.

Copy link
Contributor

reaperrr commented Mar 10, 2018

Needs rebase.

@pchote pchote force-pushed the pchote:fix-windows-unresolve-path branch from b24fcbb to 0c79ea2 Mar 10, 2018

@pchote pchote removed the PR: Rebase me! label Mar 10, 2018

@pchote

This comment has been minimized.

Copy link
Member Author

pchote commented Mar 10, 2018

Rebased.

@reaperrr reaperrr merged commit be96baf into OpenRA:bleed Mar 11, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@reaperrr

This comment has been minimized.

Copy link
Contributor

reaperrr commented Mar 11, 2018

@pchote pchote deleted the pchote:fix-windows-unresolve-path branch Apr 28, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.