Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.
Sign upWorld data and quest action development for mods #1599
Conversation
…block json
Replaces entire block, no caching for variants
… things go wrong
Also, enables multiple quest markers to be used to place resources at the target building interior using marker indexes. The first will become the normal selected marker... e.g. quest objective.
…for individuals
ClickedFoe - from DFIronman KillFoe - required a death trigger added to foe class PayMoney - allows paying with LoCs etc
This comment has been minimized.
This comment has been minimized.
@Interkarma I want to call out this change to you, out of all the changes I've made this one is the most likely to break classic quests. I do think it's a bug here though and it caused my quest to not detect the player had an item when picking off a corpse as the UID was different. I noticed that toting action checked the item quest class rather than the DaggerfallUnityItem and figured it was probably a mistake here to do so. |
This comment has been minimized.
This comment has been minimized.
Interkarma
replied
Nov 3, 2019
I agree using Contains(Item questItem) overload is the most robust method to use here. Not sure why I used the Contains(DaggerfallUnityItem item) overload. Most likely an oversight. I don't believe this will break any other quests and should test fine for all past and future quests. The check is using quest UID and item Symbol which is how it's supposed to work. The instanced DaggerfallUnityItem itself might get moved, destroyed, and recreated throughout the quest, which can lead to the item UID changing. That's why there's a quest-specific overload in the first place. The only problem I can think of is if the item is made permanent via MakePermanent action. This lowers the IsQuestItem flag so the quest item will no longer test with HaveItem. But I don't think that will affect any classic quests, and MakePermanent should only be run once the quest is finished with the item. Short version: reasonably confident this will be OK. We just need to test some other quests using HaveItem just to be sure I haven't forgotten something. |
This comment has been minimized.
This comment has been minimized.
Cool, that's what I thought / hoped, but wanted to call it out and check. Massive PR incoming. (I'm sorry I know you were enjoying a break) |
This comment has been minimized.
This comment has been minimized.
I'd love some help testing this one @TheLacus, @petchema, @numidium, @JayH2971, @Nystul-the-Magician, and any others interested. The more eyes on it before merge, the more likely we are to catch any issues. We have different areas of interest and expertise, and we could all spot something that I might miss. Thank you @ajrb for the PR and everyone who can help test. :) |
This comment has been minimized.
This comment has been minimized.
Would anyone be able to make a Linux build with this PR included? I can't make builds myself and I think this is important to test. (I'm still not free from stomach flu so this isn't urgent, but I'm happy to get involved when I can.) |
This comment has been minimized.
This comment has been minimized.
@JayH2971 Can do Jay. I'll PM you a link within next 24 hours. Thank you for help. :) |
This comment has been minimized.
This comment has been minimized.
I have elevated this PR to a branch of it's own. Merging changes to branch where we can continue work on it. |
This comment has been minimized.
This comment has been minimized.
I plan to take a look as well soon ;) |
This comment has been minimized.
This comment has been minimized.
So far I've done the following quests and picked up/delivered/dropped (where applicable) the quest item, and things have all gone well: The Courier was the one that I thought could cause problems since I surrendered the item to the thief and then killed to get it back. But the item ID persisted fine; the turn-in was a success. |
This comment has been minimized.
This comment has been minimized.
I tested a bit - so far things are working |
This comment has been minimized.
This comment has been minimized.
@ajrb I'm going to merge this and drop builds with this change over weekend. Just letting you know in case there are some last-minute changes you need to make. :) |
This comment has been minimized.
This comment has been minimized.
@Interkarma Great, sooner the better due to my upcoming lack of time. I have some this weekend and maybe a little in the coming week. In case there are issues. |
ajrb commentedNov 4, 2019
•
edited
Whew, this is a big one. I apologise in advance for the amount of effort this will take to review and check. It's going to allow some pretty cool stuff from modders though. I've been developing a new quest for the roleplay mod which has multiple paths, new npcs, new location, and dynamically changes the world providing a new service to successful players. These are all the changes I needed in core to make it possible. If anyone wants to test these changes with the new quest, let me know.
So here's a rundown of what's in this PR: