Wales and Scotland name change #3746
Conversation
| @@ -64,7 +64,7 @@ drive_on_right = { | |||
| ["South Africa"] = "false", | |||
| ["Sri Lanka"] = "false", | |||
| ["Suriname"] = "false", | |||
| ["Scotland"] = "false", | |||
| ["Alba / Scotland"] = "false", | |||
There was a problem hiding this comment.
doesnt this mean that someone converting an older planet will also get screwed? i think the compatible fix would be to include both variants right? just support both for a time. its a bit of baggage to carry but its better than getting bug reports :D
| @@ -191,7 +191,7 @@ function rels_proc (kv, nokeys) | |||
| end | |||
| end | |||
| end | |||
| if kv["name"] == "England" or kv["name"] == "Scotland" or kv["name"] == "Wales" or kv["name"] == "Northern Ireland" then | |||
| if kv["name"] == "England" or kv["name"] == "Alba / Scotland" or kv["name"] == "Cymru / Wales" or kv["name"] == "Northern Ireland" then | |||
There was a problem hiding this comment.
doesnt this mean that someone converting an older planet will also get screwed? i think the compatible fix would be to include both variants right? just support both for a time. its a bit of baggage to carry but its better than getting bug reports :D
@kevinkreiser Yeah I had the following, but was concerned folks would complain. Thoughts?
if kv["name"] == "Alba / Scotland" then
kv["name"] = "Scotland"
end
if kv["name"] == "Cymru / Wales" then
kv["name"] = "Wales"
end
There was a problem hiding this comment.
I had actually meant just leave both checks in and use whichever we find. Which means doing it at the other places where you have changes as well. Basically support both names all the way through the code
There was a problem hiding this comment.
@kevinkreiser If I don't do the lua change I posted, then we will have to have support for both here too https://github.com/valhalla/valhalla/pull/3746/files#diff-687f31f02c6bb0a624827f33f233de7302f94e2c5c1099b33931752ff40d94aaR135
There was a problem hiding this comment.
Yep I know that's what I'm suggesting
There was a problem hiding this comment.
Do we know when those relations changed? If it was a long while ago then just ignore my comment. If it was recent then it's a pretty huge gotcha to have to know to use the bleeding edge changeset
There was a problem hiding this comment.
@kevinkreiser Scotland was 8 months ago and Wales I am not sure as it keeps timing out on me.
There was a problem hiding this comment.
ok cool then we can just ship it as is. we'll assume wales was also a while ago. sorry for the noise i just didnt want to knowingly put a gotcha in there
There was a problem hiding this comment.
@kevinkreiser np. thanks for the feedback
|
urgh.. windows fails with some vcpkg idiocy.. can someone pls re-trigger that workflow, hoping it's just a hiccup? lint build is the usual coverage fail |
gknisely
dismissed stale reviews from kevinkreiser and nilsnolde
via
51a6c2e
Issue
Wales and Scotland had a name change. We need to come up with a better design and handle name changes. Not sure what to do yet. Anyhow #fixes #3135 Added a small test showing how just one user inputted access can shut off country access overrides.
Tasklist
Requirements / Relations
Link any requirements here. Other pull requests this PR is based on?