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

Made location_area identifiers actually mean something to a human #1010

Conversation

DaltonSW
Copy link
Contributor

Added region and location information to the location_area identifier fields so it means something to a human reading looking at them

@Naramsim
Copy link
Member

Hi, we construct the name starting from the location name. So for the ID 1 you can see the name property correctly set.

If we merge this PR the first part of the name would be doubled. Ending up in sinnoh-canalave-city-canalave-city-area.

Nonetheless I think we can change some identifiers:

  • all galar-*. Example. In the current API the name is rendered as galar-route-1-galar-1. So, of your PR I'd keep the route addition.
  • remove all galar- prefixes

@DaltonSW
Copy link
Contributor Author

Okay, I see what you mean. Is there any place where this kind of stuff is documented? This lack of specific documentation or any references of how the CSVs interact is part of what I was trying to address with this change

For example, if there was something saying" the location area identifier is used to construct the location endpoint name element", I would be able to more specifically target my testing and verifying

@DaltonSW
Copy link
Contributor Author

How would you feel about an extra, optional column being added to some of the CSVs to act as a human-readable descriptor? Some of the barriers to me adding more info to these files is that I have no idea what the hell I'm looking at without busting out a whiteboard or a script to mentally piece everything together 😅

@DaltonSW DaltonSW closed this Jan 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants