[0.11.0] Travel Map: Location names don't take WorldData overrides into account [RESOLVED 0.11.2]

User avatar
Hazelnut
Posts: 3015
Joined: Sat Aug 26, 2017 2:46 pm
Contact:

Re: [0.11.0] Travel Map: Location names don't take WorldData overrides into account

Post by Hazelnut »

Sorry in advance if this post is disorganised, it's really hard when a conversation goes back and forth while I'm asleep and so I don't have a chance to interject... anyway I wanted to respond and have now taken some time to dive back into the code I wrote and hopefully you will both get my points clearly. :)


The location overrides are quite limited in scope, and like all world data override there's no localisation intended. I've assumed that DF place names, generated names etc would not be part of the localisation system. It's very common for names to not be translated, and it's only really even possible for names with meanings anyway. (I mean what would you translate Pjiga to exactly?) If it were my decision I would not support localisation for reagion or location names and that's what I've assumed would be the case.

Regarding the concept of world data override system, all they do is provide a different source for the DF maps data, with different levels of granularity (location, block, building) and allow some of the data to be changed. For locations the region name should match the region the location is in. There's no overriding or changing the regions, they are too fixed to attempt even if I wanted to. (which I don't)

The placement of locations in the world is already something that can be changed and new ones put wherever, as long as it's within the region boundaries of course.
The latitude/longitude are integral to world indexing, this would lean more into adding new locations or moving locations than just replacing existing data. I don't feel this is something I'd want to support without serious review, if ever.

Region names are hardcoded and immutable. There are methods that search for locations by name using strings for both region and location, so we need to be careful with this. While researching feasibility during initial localization rollout, I decided best not to touch region names for now. Region names are proper (fantasy) nouns in any case, and shouldn't require translation. There's a strong chance they will not enter localization.

However, locations might require translated names, or even just corrections like fixing "THe Crypts of Rhajom". I'm still considering how location names will be supported in localization framework, and how this will interact with world data replacements.

I'm more open to changing location type however. I think this will work well enough for purposes of travel map filtering and could be used to "convert" a location into something similar but different - e.g. convert a temple to a custom coven.
So location lat/long can already be used to move existing or place new locations so you did already support this when you accepted my PR - sorry if you didn't feel like you realised so you could give it a serious review. There may be some scope for overrides that move existing locations to break savegames, but it's possible if a mod author wants to do that. No one has tried to do this, including me so maybe it doesn't work for existing locations... it's not really an intended use case of the system. So it's not supported exactly but it is technically possible, in the same way that typing 'wibble' into the location type field is also possible. It was never intended as a fool proof system, and modders need to know what they're doing and think about stuff or bugger things up.

Anyway the override system was fully integrated with the travel map, that was one of the big things I had to achieve when allowing locations to be created and it's used in my Mountain Rumors quest. The exceptions are that I'd missed that modified existing locations might change/fix the name and that existing locations pre-discovered flag might be altered - both sort of edge cases but valid and you've now fixed those. New locations were not affected by either of these issues, only modified existing locations.

So reviewing the gaps in code you've fixed, because I'm pretty rusty, seems I added a call to GetDFRegionAdditionalLocationData() in MapsFile.ReadRegion() when it generates the region location name list, which handled the new locations but never did anything for existing location name. The solution you've implemented is fine Interkarma, but I suspect had I done the fix that I'd have added a method to update the DFRegion MapNames list like GetDFRegionAdditionalLocationData() does for new locations when a region is loaded, however it's just as valid to do it the way you have on demand. The one thing I'd point out is that any other code using region location name list will have to do the same, rather than having the list consistent from the source - the region read/parse.

Similarly, ContentReader.EnumerateMaps() could also get the correct discovery flag for modified existing locations if MapsFile.ReadRegion() checked for overrides when generating the MapTable. Same considerations as above.

If you want I could re-fix these to ensure that world data overrides are not leaking into a UI window and are handled correctly in MapsFile. Depends if it bothers you because what you've done is functionally fine. I think I would slightly prefer it, but happy to discuss pros/cons if you wish. (when I reviewed your fixes I only considered functionality and not approach, because I'm a bit rusty - I only dived into things more after reading last nights exchanges)

I'm not talking about renaming regions. I'm talking about the RegionName parameter on the 4th line of a location override. That said, now that I think about it, I'm not sure what the game would use this piece of data for in the first place. The region a location is found in is determined by the 3 character extension attached to the 4 MAPS.BSA records of each region. And according to the UESP, this parameter doesn't appear to be present in the original data. So is this just added by DFU for identification purposes?
This is true, DFU added it into the internal location data structure to optimise code working with locations so it could quickly access the region name (I assume, IK can confirm/deny) and it's marked as public so it gets serialised into exports. This could be marked internal and the reading of location overrides could reconstruct it, but I tended to only move fields from internal to public and leave the rest of the map data structures alone. I'll only do this if Interkarma wants me to, otherwise you will need to ensure you put the right region name in here. This would have to change if region names are to be localised, but as I already said I don't expect us to be doing that. (could be wrong tho)

I believe region name is serialized only for easy identification in JSON file. Region name in world data replacement doesn't map back to anything in game that I'm aware of.
No, I think that DFLocation.RegionName field is used all over the code. See my previous paragraph.


tl;dr - summary

1) Do we really want to localise region and location names? I think we shouldn't and have assumed we wouldn't up to now.

2) Should I re-do the two fixes to be consistent with how I implemented the rest of the WD override system so that the map data reading and parsing code handles it all behind the scenes, and UI windows etc don't need to know anything about WD overrides?
See my mod code for examples of how to change various aspects of DFU: https://github.com/ajrb/dfunity-mods

User avatar
Interkarma
Posts: 7236
Joined: Sun Mar 22, 2015 1:51 am

Re: [0.11.0] Travel Map: Location names don't take WorldData overrides into account

Post by Interkarma »

I'm not sure if folks outside of IT use "unsupported" in the same way we do - what I'm saying is "this is likely a bad idea and I will not support you if it breaks". If it's not an intended use of the system, then it's unsupported. That doesn't mean you can't try, or even that it won't work. It means you're doing something on your own and you won't get any help if it goes wrong.

It's possible display name of regions will enter localization at some point, but this has some issues I don't want to tackle just yet. I do plan on adding location display names to localization. Some of the classic translation packs already do this by way of hacking MAPS.BSA. I'd love to support this properly for our international community with string tables. I'm perfectly OK with it if world data replacements doesn't want to dovetail with this in any way and simply overrides the name.

Please don't take my earlier comments as an endorsement of intent, I was only noodling around concepts with XJDHDR. I think I'll cease noodling on the forums in future. this behaviour always seems to cause discussion to expand rather than contract, and I just don't have that kind of time.

If you'd like to redo those fixes, I'm OK with that. I mainly jumped into this conversation because it impacted something in the base game and I wanted to offer help as one of the original authors of travel map. Happy for this to be done in a different and better way.

The two issues with display names and initial discovery on travel map are resolved for now. That's good enough for me so far as this bug report is concerned.

User avatar
Hazelnut
Posts: 3015
Joined: Sat Aug 26, 2017 2:46 pm
Contact:

Re: [0.11.0] Travel Map: Location names don't take WorldData overrides into account

Post by Hazelnut »

Interkarma wrote: Tue Feb 02, 2021 10:36 pm I'm not sure if folks outside of IT use "unsupported" in the same way we do - what I'm saying is "this is likely a bad idea and I will not support you if it breaks". If it's not an intended use of the system, then it's unsupported. That doesn't mean you can't try, or even that it won't work. It means you're doing something on your own and you won't get any help if it goes wrong.
Ah, sorry, my misunderstanding.. we're actually in alignment on it then. Sorry for confusion there, never having done IT support I think of what is supported as what the software will do if you tell it. :D
Interkarma wrote: Tue Feb 02, 2021 10:36 pm It's possible display name of regions will enter localization at some point, but this has some issues I don't want to tackle just yet. I do plan on adding location display names to localization. Some of the classic translation packs already do this by way of hacking MAPS.BSA. I'd love to support this properly for our international community with string tables. I'm perfectly OK with it if world data replacements doesn't want to dovetail with this in any way and simply overrides the name.
My assumption was wrong there,and I guess you're more generous to the international community than I am. (sorry international community :oops:) Anyway, given that TheLacus is already developing mod localisation, I feel that if you do allow translations of region/location names then I would definitely want to try to ensure WD overrides can support it as well.
Interkarma wrote: Tue Feb 02, 2021 10:36 pm If you'd like to redo those fixes, I'm OK with that. I mainly jumped into this conversation because it impacted something in the base game and I wanted to offer help as one of the original authors of travel map. Happy for this to be done in a different and better way.
Nah, I'm good thanks. :) Would have done it if you wanted me to after explaining but I think the fixes are fine as they are for now and I appreciate you doing them since I do have limited time atm. (though I am trying to do more around here when I get the opportunity like last few days) If we do circle round and localised region/location names are on the cards, then I'll reconsider at that point if I think it would make anything easier or simpler to implement.

Anyway, apologies for the wall of text. I probably should have waited until I finished work and made a more streamlined and organised response for you. As my wife says, I can be a bit full on when I react quickly to stuff at times... still we just celebrated 28 years together so I must be doing something right! :D


@XJDHKR, I think it's great what you're doing with the system and that it's giving the it such a good work out and fixing/raising issues. Having people really use it so comprehensively makes all the time spent on it feel worthwhile.
See my mod code for examples of how to change various aspects of DFU: https://github.com/ajrb/dfunity-mods

User avatar
Interkarma
Posts: 7236
Joined: Sun Mar 22, 2015 1:51 am

Re: [0.11.0] Travel Map: Location names don't take WorldData overrides into account

Post by Interkarma »

Hazelnut wrote: Tue Feb 02, 2021 11:19 pm Ah, sorry, my misunderstanding.. we're actually in alignment on it then. Sorry for confusion there, never having done IT support I think of what is supported as what the software will do if you tell it. :D
Haha you wouldn't believe some of the things people manage to make software do in my world that I have to fix afterwards. Some people get very creative. :lol:

Hazelnut wrote: Tue Feb 02, 2021 11:19 pm Anyway, given that TheLacus is already developing mod localisation, I feel that if you do allow translations of region/location names then I would definitely want to try to ensure WD overrides can support it as well.
We'll cross that bridge when we come to it. I think I'll be working on book text overrides from localization next, then quest. Lots of lead time before we have to worry about locations.

Hazelnut wrote: Tue Feb 02, 2021 11:19 pm @XJDHKR, I think it's great what you're doing with the system and that it's giving the it such a good work out and fixing/raising issues. Having people really use it so comprehensively makes all the time spent on it feel worthwhile.
Seconded. I love watching you guys do all the things you do.

User avatar
XJDHDR
Posts: 258
Joined: Thu Jan 10, 2019 5:15 pm
Location: New Zealand
Contact:

Re: [0.11.0] Travel Map: Location names don't take WorldData overrides into account

Post by XJDHDR »

Interkarma wrote: Wed Feb 03, 2021 1:29 am
Hazelnut wrote: Tue Feb 02, 2021 11:19 pm @XJDHKR, I think it's great what you're doing with the system and that it's giving the it such a good work out and fixing/raising issues. Having people really use it so comprehensively makes all the time spent on it feel worthwhile.
Seconded. I love watching you guys do all the things you do.
You're both welcome.

As for Interkarma's suggested alteration to the JSON data, I don't personally see any problems with doing it that way.

Locked