implementation of talk window

Discuss coding questions, pull requests, and implementation details.
User avatar
Interkarma
Posts: 3600
Joined: Sun Mar 22, 2015 1:51 am

Re: started implementation of talk window

Post by Interkarma » Sat May 05, 2018 10:46 pm

It seems I already have a static method to determine if a building is a residence. So rather than comparing name != "Residence", you can use !RMBLayout.IsResidence(). :)

User avatar
Nystul
Posts: 1185
Joined: Mon Mar 23, 2015 8:31 am

Re: started implementation of talk window

Post by Nystul » Sat May 05, 2018 10:49 pm

Interkarma wrote:
Sat May 05, 2018 9:57 pm
Nystul wrote:
Sat May 05, 2018 9:24 pm
(trusting that the name "Residence" will be used as internal mechanism to hide)
It occurs to me this probably isn't the best way for us to do this. When it becomes possible to translate some of these default labels, the following code will break.

Code: Select all

// only create nameplates for buildings different than "Residence"
if (discoveredBuilding.displayName != "Residence")
	newBuildingNameplate.name = discoveredBuilding.displayName;
I'd prefer it if the residence was filtered in the following way:

Code: Select all

if (!IsResidence(discoveredBuilding.buildingType))
	newBuildingNameplate.name = discoveredBuilding.displayName;
In the above, the new method IsResidence() would check discoveredBuilding.buildingType and return true if building is one of the known residence types. This is much more data-stable than just checking for text string "Residence".

To expand on the above for quests. I'd prefer it if name plates are only shown for residences when involved in an active quest. Once the quest ends, the nameplate would be filtered out again. Can probably rework above to something like the following.

Code: Select all

// Only show nameplate if residence is involved in a quest
string buildingQuestName = GetBuildingQuestName(discoveredBuilding.buildingKey);    // Returns string.Empty if building not involved in quest
if (IsResidence(discoveredBuilding.buildingType) && !string.IsNullOrEmpty(buildingQuestName))
	newBuildingNameplate.name = buildingQuestName;
That way it should not be necessary to do any resetting of name plates. It will inherit name directly from quest only when a quest is operating on that residence, and otherwise not show anything for simple residences not involved in any quest work currently. I can help create an efficient means of retrieving quest name from buildingKey if needed.

From a design point of view, I prefer active state to come in from active systems. Meaning the automap should be a consumer of state rather a provider of state. It shouldn't be making decisions about what buildings are called and when to reset names. It should just show what they're called by combining intelligence from other systems.
ok, yeah much cleaner - will change it and test it out! ;)
Interkarma wrote:
Sat May 05, 2018 10:13 pm
Nystul wrote:
Sat May 05, 2018 5:00 pm
failure inside function DiscoverBuilding in PlayerGPS because:
in invoked GetBuildingDiscoveryData function GameManager.Instance.StreamingWorld.GetCurrentBuildingDirectory() fails because currentPlayerLocationObject inside it is null (because no externals has been loaded)
Just on this, why do you need to call DiscoverBuilding() again? The building should have been added permanently to discovery data at the moment player entered it from the exterior. And the discovery data is always available from PlayerGPS even when loading a fresh interior save. There should not be any need to "rediscover" the building. This is all automatic.

I might need more detail here, as I'm not sure we're solving the right problem. If this is a workaround to change names in the discovery system, that would be the wrong approach for the design. You should never need to modify the discovery state. My previous post has some other suggestions. :)
my understanding was that I have to call GameManager.Instance.PlayerGPS.DiscoverBuilding(buildingKey, nameDerivedFromPlaceResourceSiteDetails) to update the dl.discoveredBuildings[db.buildingKey].displayName value - it did not show up otherwise for me (but I will check again) - in the same way I thought I would have to undo the displayName change - maybe I don't understand it correct and this mechanism should already work by itself without additional calls to DiscoverBuilding - but then I need to test why it did not show up automatically

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

Re: started implementation of talk window

Post by Interkarma » Sat May 05, 2018 11:08 pm

Nystul wrote:
Sat May 05, 2018 10:49 pm
my understanding was that I have to call GameManager.Instance.PlayerGPS.DiscoverBuilding(buildingKey, nameDerivedFromPlaceResourceSiteDetails) to update the dl.discoveredBuildings[db.buildingKey].displayName value - it did not show up otherwise for me (but I will check again)
The name override was added by Hazelnut for his guild work. It diverges from my design a little, but I get why he had to do it for Thieves and Dark Brotherhood. They need to modify permanent record.

We shouldn't have to set an override name here for residences used in quests, as we don't want to edit permanent record. Rather we should try to use my hypothetical GetBuildingQuestName(discoveredBuilding.buildingKey) just to see if building is temporarily involved in a quest. I can help build this if it gets things off the ground faster.
Nystul wrote:
Sat May 05, 2018 10:49 pm
- in the same way I thought I would have to undo the displayName change - maybe I don't understand it correct and this mechanism should already work by itself without additional calls to DiscoverBuilding - but then I need to test why it did not show up automatically
We shouldn't need to do this. The building discovery is intended to be a permanent record generated automatically by player simply moving through world. At least, that was my intended design. Any name changes created by quests should only temporarily "replace" the default building name while quest is active. And this should be done through being "quest aware" (pull replacement name from quest system) rather than modifying the discovery database back and forth.

User avatar
Nystul
Posts: 1185
Joined: Mon Mar 23, 2015 8:31 am

Re: started implementation of talk window

Post by Nystul » Sat May 05, 2018 11:13 pm

ok, just leave my pull request open for the moment - don't close it - I will update and fix all this ;)

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

Re: started implementation of talk window

Post by Interkarma » Sat May 05, 2018 11:16 pm

All good mate. And just let me know where I can help if needed, I'm always available to you. :)

User avatar
Nystul
Posts: 1185
Joined: Mon Mar 23, 2015 8:31 am

Re: started implementation of talk window

Post by Nystul » Sat May 05, 2018 11:19 pm

thanks! :) let's see how far i get

edit:
have it working - was much easier as expected
for now I implemented the functionality direct in DaggerfallExteriorAutomap.cs, this should better be a own function call in the future:

Code: Select all

                                if (!RMBLayout.IsResidence(buildingSummary.BuildingType)) // guilds, shops, taverns, palace (general case)
                                {
                                    newBuildingNameplate.name = discoveredBuilding.displayName;
                                }
                                else if (RMBLayout.IsResidence(buildingSummary.BuildingType)) // residence handling
                                {
                                    // check if residence is involved in active quest (this might be replaced by function call - for now I implemented this by myself
                                    string buildingQuestName = string.Empty; // string.Empty if building not involved in active quest
                                    ulong[] questIDs = GameManager.Instance.QuestMachine.GetAllActiveQuests();
                                    foreach (ulong questID in questIDs)
                                    {
                                        Quest quest = GameManager.Instance.QuestMachine.GetQuest(questID);
                                        QuestResource[] questResources = quest.GetAllResources(typeof(Place)); // get list of place quest resources
                                        foreach (QuestResource questResource in questResources)
                                        {
                                            Questing.Place place = (Questing.Place)(questResource);
                                            if (place.SiteDetails.buildingKey == discoveredBuilding.buildingKey)
                                            {
                                                buildingQuestName = place.SiteDetails.buildingName; // get buildingName if involved in active quest
                                            }
                                        }
                                    }

                                    // only show nameplate if residence is involved in a quest
                                    if (!string.IsNullOrEmpty(buildingQuestName))
                                        newBuildingNameplate.name = buildingQuestName;
                                }

User avatar
Nystul
Posts: 1185
Joined: Mon Mar 23, 2015 8:31 am

Re: started implementation of talk window

Post by Nystul » Sat May 05, 2018 11:33 pm

feel free to give the new implementation a try ;) should work now

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

Re: started implementation of talk window

Post by Interkarma » Sun May 06, 2018 12:03 am

Yeah, that looks like it should work. Good job! :)

We can possibly improve the efficiency of testing every residence against every Place resource as well. Maybe with an easy to query dictionary of active Place resources by buildingKey in QuestMachine. Although in practice there's probably only a few (about 1-5) Place resources active at any time, so the savings probably won't be that great.

User avatar
Nystul
Posts: 1185
Joined: Mon Mar 23, 2015 8:31 am

Re: started implementation of talk window

Post by Nystul » Sun May 06, 2018 12:07 am

yeah - would be happy to integrate whatever solution we will come up with ;)
as soon as we have a dedicated function I will drop the current placeholder implementation for the improved one

fyi: it is really a good feeling if you play through a dialog heavy quest with several entries in the "Tell Me About" section and several person and location resources and everything works and all of them get removed after quest completion ;)
let's wait for the first bug reports though :P

btw, from my side pull request is ready now for your review, will put a timer on the "where is - work" questor creation tomorrow (creation is done now on location change/loading savegame) - pretty late here now

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

Re: started implementation of talk window

Post by Interkarma » Sun May 06, 2018 12:32 am

Cheers, I'll try to get that merged today sometime.

I know that feeling of satisfaction you're talking about. There's all these little systems finally just clicking together. It's starting to feel like the real deal in almost every way now.

Post Reply