Looking for some feedback on these dialogues for my mod

[Daeros_Trollkiller]

Heya folks, as I am working on the rewrite for my mod, I've had to change the way I did items as quest rewards for the new way I am importing SoD items to work properly.  They are working as I intended them to, but I haven't done much with dialogues, so I would appreciate some feedback on how I did these to see if I made any glaring mistakes that might cause issues.  In case it matters: I had my registered prefix changed to "K0" from "DTK".  Having that one extra character to work with really helps more than I thought it would.

Example 1

EXTEND_BOTTOM VAELASA 4

  IF ~HasItem("bdbrd01",Myself)~ THEN DO ~AddexperienceParty(32500)
TakePartyItem("misc4n")
GiveItem("bdbrd01",LastTalkedToBy)
SetGlobal("SummonDryads","AR1200",1)
~ EXIT
END

Example 2

EXTEND_BOTTOM UDSOLA01 161

IF ~Global("BD_HAVE_SOD","GLOBAL",1)
    Global("K0_SOD_IMPORT_75","GLOBAL",1)~ THEN GOTO K0SODIMP
END

APPEND UDSOLA01

IF ~~ THEN BEGIN K0SODIMP
    SAY @80003

IF ~~ THEN DO ~GiveItemCreate("bdbrd02",LastTalkedToBy,1,0,0)
SetGlobal ("K0_SOD_IMPORT_75","GLOBAL",2)
EscapeAreaObject("Tran2100")
~ EXIT
END
END

 

Edited April 26 by Daeros_Trollkiller

[jastey]

Indeed, there are some basic, nonwritten rules that were established for dialogue patching in IE modding over the decades. In short: Make it as compatible and non-intrusive as possible. The reason is simple: another mod might have changed the transaction you are aiming at, meaning your mod would overwrite the other mod's changes. And - another mod might change the state your mod did changes to - overwriting yours. weidu was developped to make additions to dialogues as compatible as possible.
This is done by:
1. Always patch your new transactions into the existing one. Do not add a new transition that includes "hard copied" transactions into your own mod. What I mean with hard copied is: copying the transactions that are in the original game and paste them into your mod package. Like I said, another mod might have already added to it, or a tweak mod might have changed the reward, etcpp. Inserting hard copied transactions would erase all changes the player already installed to their game.

2. If you make up a new branch from an existing dialogue state, always circle back to its transitions using COPY_TRANS. Otherwise, your branching off will erase any additions or changes other mods added - or yours might be overwritten by another mod installed later if it doesn't reflect this.

There is more like "Never double a dialogue state just because you wanted to change the text line or change the transitions but patch the existing instead" etcpp.

To your examples:
The first one to VAELASA violates no. 1. In my unmodded game VAELASA 4 has the transactions:

~AddExperienceParty(32500)
TakePartyItem("misc4n")
SetGlobal("SummonDryads","AR1200",1)~

Apparently, your mod should add ~GiveItem("bdbrd01",LastTalkedToBy)~.
So, instead of overwriting the existing transaction of VAELASA 4 by using EXTEND_BOTTOM like you did, you'll patch it simply by:

Dang, I see now that you have a trigger there. What I suggest here would work, but it's a bit dirty. See below for comments.

ADD_TRANS_ACTION VAELASA BEGIN 4 END BEGIN END ~GiveItem("bdbrd01",LastTalkedToBy)~

Big bonus: this will also work in case another mod added a whole new transition.

The Solaufein addition violates no. 2 and 1: again it includes the existing game transcation, which is ~EscapeAreaObject("Tran2100")~, which could be changed by another mod. Also, what if another mod wants Solaufein to say another line here, too? If your mod is installed, the content of the other mod will be erased - or another mod doing it the same way would overwrite yours.
So, branching with EXTEND_BOTTOM to a new state is fine, but you need to close the loop using COPY_TRANS, which will add the transactions of the original state like they are in the players' game the moment your mod is installed to your new state, letting the player experience all transactions that were there before. This needs a little restructure so you can put your new actions in:

EXTEND_BOTTOM UDSOLA01 161
IF ~Global("BD_HAVE_SOD","GLOBAL",1)
    Global("K0_SOD_IMPORT_75","GLOBAL",1)~ THEN DO ~GiveItemCreate("bdbrd02",LastTalkedToBy,1,0,0)
SetGlobal ("K0_SOD_IMPORT_75","GLOBAL",2)~ GOTO K0SODIMP
END

APPEND UDSOLA01
IF ~~ THEN BEGIN K0SODIMP
    SAY @80003
    COPY_TRANS UDSOLA01 161
END
END

 

[jastey]

Ah, I see you have an additional trigger in the VAELASA transaction. Let me ponder about how I'd do this.

[jastey]

OK, I'd do the first one differently so it doesn't scramble whatever the player already patched to their VAELASA 4 state. Since the item should only be handed if the dryad actually has it, i.e. there is an additonal condition and transaction, I do not see an easy way to do this via just patching VAELASA 4.

Possibility 1: add a script block to the area script / dryad's script that gives the item "bdbrd01" to the player in case the dryad has it in its inventory as soon as the variable "Global("SummonDryads","AR1200",1)" from her original dialogue is set.

Possibility 2: do it like you did for Solaufein, adding a new text line (+ dlg state) and COPY_TRANS.

Possibilit 3: make the engine limitation your feature. Items will only be handed via GiveItem if the character talking has them in inventory. If the item is not there, the action will just be skipped. So, if the dryad doesn't have it then she will not give it, simply put. Then, in this case, you can use what I suggested above using ADD_TRANS_ACTION.

[Daeros_Trollkiller]

Thank you for the feedback, Jastey.  Could you expand more on what you mean on #3?  I thought that I would need to have a trigger: HasItem, to check if an NPC actually has something before using GiveItem.  I thought it might cause issues if I didn't.  Are you saying that using GiveItem by itself is fine, even if the NPC doesn't have the item in it's inventory to give?  Hell, I figured it might crash the game if it tried to give an item it didn't have.

For #2 - based on your feedback, that's the one I think I will rely on the most then, unless I want the NPC to be able to use the item in combat AND give it as a quest reward, which is why I did Vaelasa the way I did.  I had a feeling I would need a COPY_TRANS in there, but wasn't sure, and not sure where.  I really appreciate you taking your time to explain all this to me.

 

[jastey]

6 hours ago, Daeros_Trollkiller said:

Hell, I figured it might crash the game if it tried to give an item it didn't have.

Hey, come on! This is the IE Engine! Just because it crashes if there is a wrongly formatted text or a "wrong" animation color it doesn't mean it would just crash here, right? Right?

On a serious note: this should indeed be fine. It's actually content of bug reports: quest giver xyz didn't notice I already stole the item from his inventory etcpp. You could always test to make sure. I'm 99.95% sure it will not lead to a crash if the item is no longer in inventory.

COPY_TRANS only works if you added a new dialogue state you can add it to. With a new state like you did for Solaufein, it's straight forward.

Cheers!

And, I really appreciate you asking this and collecting knowledge. I know I should write more tutorials about this, so the blame is on me / us older modders for not passing it on properly, but if I see newer mods using the incompatible methods it aches my heart.

[Daeros_Trollkiller]

I have another one I wanted to run by you more experienced folks:

This one involves me trying to use a new trigger to have the genie in trademeet reward a SoD scimitar in place of the normal one.  Everything works, just want to see how COPY_TRANS would apply here, if it's needed, or something else altogether:

EXTEND_BOTTOM TRGENI01 35

IF ~Global("BD_HAVE_SOD","GLOBAL",1)
    Global("K0_SOD_IMPORT_30","GLOBAL",1)~

THEN DO ~GiveItemCreate("bdsw1h08",LastTalkedToBy,0,0,0)
         SetGlobal ("K0_SOD_IMPORT_30","GLOBAL",2)~ GOTO 36
END

EXTEND_BOTTOM TRGENI01 39

IF ~Global("BD_HAVE_SOD","GLOBAL",1)
    Global("K0_SOD_IMPORT_30","GLOBAL",1)~

THEN DO ~TakePartyItem("misc8k")
         GiveItemCreate("bdsw1h08",LastTalkedToBy,0,0,0)
         SetGlobal ("K0_SOD_IMPORT_30","GLOBAL",2)~ GOTO 36
END

And regarding COPY_TRANS.  What if all I want to do with a transaction is add a new trigger that routes to a new state that includes everything from the original, and just adds a new string to it?

For instance here:

IF ~~ THEN BEGIN 54 // from: 53.0
  SAY #36309 /* ~Your place is here whenever you wish it, even if you do not actively serve. Remember that you remain in the gaze of the Vigilant One, and that your actions reflect upon him. Go with honor.~ */
  IF ~~ THEN DO ~AddexperienceParty(35000)
GiveGoldForce(1000)
SetGlobal("Talonwar","GLOBAL",3)
EraseJournalEntry(55617)
EraseJournalEntry(55619)
EraseJournalEntry(55621)
EraseJournalEntry(55622)
EraseJournalEntry(55623)
~ SOLVED_JOURNAL #55618 /* ~Cleric stronghold: Halting the Talon war

I have served the greater law and pacified both the Talons and the followers of the Morninglord. My service to Helm has been recognized as invaluable. For now, I have been released from further service as a boon for all that I have done so far. I await to be called if need be.~ */ EXIT
END

What I would like to be able to do here is have an additional trigger:

IF ~Global("BD_HAVE_SOD","GLOBAL",1)
    Global("K0_SOD_IMPORT_46","GLOBAL",1)~

which would do the below in addition to what normally happens, without any additional SAY's and such:

THEN DO ~GiveItemCreate("bdblun09",LastTalkedToBy,0,0,0)
SetGlobal ("K0_SOD_IMPORT_46","GLOBAL",2)~

If such a thing is even possible to do in a way that doesn't cause issues for other mods.

Edited April 29 by Daeros_Trollkiller