TangerineDog Posted March 27, 2020 Share Posted March 27, 2020 (edited) I'd be grateful if you could help me. I'm adding a spell to the player that checks for certain items in an opponent's inventory when he hits the player and removes one of them on each hit - I've removed all the ckecks if that opponent is actually the attacker in the code further down just to improve readability :smile: Since I'd like the order these items are removed in to be random, I thought I'd just generate a random number that determines which object is checked for.Should that item have been removed already or - also a possibility - should the opponent never have had it in the first place, the script jumps to the next item and checks for that. The problem now is that if the number is,for example, 2, and 2 has been removed already, and it checks for 3 and 3 is gone as well, the script just stops - but 1 is still there and doesn't get checked! So for this one hit, 1 could still be left to be removed but not be removed. And of course, this is because once the script has checked 2 and 3, the last if is done and I don't know how to make it jump back to the beginning of the event, so to speak. The script so far is this: [code]Scriptname CheckOnHitScript extends ActiveMagicEffectMiscObject Property MiscItem01 AutoMiscObject Property MiscItem02 AutoMiscObject Property MiscItem03 AutoEvent OnHit()int random = Utility.RandomInt(1, 3)If random == 1If (akAggressor.GetItemCount(MiscItem01) > 0)akAggressor.RemoveItem(MiscItem01, 1)Elserandom = random + 1EndIfEndIfIf random == 2If (akAggressor.GetItemCount(MiscItem02) > 0)akAggressor.RemoveItem(MiscItem02, 1)Elserandom = random + 1EndIfEndIfIf random == 3If (akAggressor.GetItemCount(MiscItem03) > 0)akAggressor.RemoveItem(MiscItem03, 1)Elserandom = 1EndIfEndIfEndEvent[/code] Another problem is that once all the items have been removed, the script shouldn't just run on and on from top to bottom, checking for items that aren't there anymore, but 've found a workaround for that that I've removed as well just to avoid having too long a text here. If you have any thoughts on how I could make the script do what I want, would you mind helping me out? Edit: I think I just solved it. How 'bout this: [code]Scriptname CheckOnHitScript extends ActiveMagicEffectMiscObject Property MiscItem01 AutoMiscObject Property MiscItem02 AutoMiscObject Property MiscItem03 AutoEvent OnHit()int random = Utility.RandomInt(1, 3)If random == 1If (akAggressor.GetItemCount(MiscItem01) > 0)akAggressor.RemoveItem(MiscItem01, 1)Elserandom = random + 1EndIfEndIfIf random == 2If (akAggressor.GetItemCount(MiscItem02) > 0)akAggressor.RemoveItem(MiscItem02, 1)Elserandom = random + 1EndIfEndIfIf random == 3If (akAggressor.GetItemCount(MiscItem03) > 0)akAggressor.RemoveItem(MiscItem03, 1)Elserandom = 1EndIfEndIf If random == 1If (akAggressor.GetItemCount(MiscItem01) > 0)akAggressor.RemoveItem(MiscItem01, 1)Elserandom = random + 1EndIfEndIfIf random == 2If (akAggressor.GetItemCount(MiscItem02) > 0)akAggressor.RemoveItem(MiscItem02, 1)Elserandom = random + 1EndIfEndIfIf random == 3If (akAggressor.GetItemCount(MiscItem03) > 0)akAggressor.RemoveItem(MiscItem03, 1)Elserandom = 1EndIfEndIfEndEvent[/code] By having it run thourgh the whole thing TWICE, no item can be missed no matter where in the list it starts and I don't need to avoid a loop either. :)Now I just need to add a variable that differentiates between the first and the second run-through to avoid an item being checked for twice in one hit. Edited March 27, 2020 by TangerineDog Link to comment Share on other sites More sharing options...
IsharaMeradin Posted March 27, 2020 Share Posted March 27, 2020 Sounds like you may have worked it out for yourself. Decided to take a stab at it myself anyway. Feel free to test this out if you want. Scriptname CheckOnHitScript extends ActiveMagicEffect MiscObject Property MiscItem01 Auto MiscObject Property MiscItem02 Auto MiscObject Property MiscItem03 Auto Event OnHit(ObjectReference akAggressor, Form akSource, Projectile akProjectile, bool abPowerAttack, bool abSneakAttack, bool abBashAttack, bool abHitBlocked) ; requries a pre-check in the event that none of the items are present. Int num = 1 While CheckMyItem(akAggressor,num) == False && num < 4 num += 1 EndWhile If num < 4 ; at least one of the items are present int random = Utility.RandomInt(1,3) Int current = random ; NOTE - did not want to modify the random number in case that specific value may be needed further on. While current > 0 && current < 4 If CheckyMyItem(akAggressor,current) == true RemoveMyItem(akAggressor,current) current = 0 ; got a valid item - exit loop Else ; try next current += 1 If current == 4 current = 1 ; if we reach the end, loop back to the beginning EndIf EndIf EndWhile EndIf EndEvent Bool Function CheckMyItem(ObjectReference SourceContainer, Int ItemNum) If ItemNum == 1 && (SourceContainer.GetItemCount(MiscItem01) > 0) Return True ElseIf ItemNum == 2 && (SourceContainer.GetItemCount(MiscItem02) > 0) Return True ElseIf ItemNum == 3 && (SourceContainer.GetItemCount(MiscItem03) > 0) Return True Else Return False EndIf EndFunction Function RemoveMyItem(ObjectReference SourceContainer, Int ItemNum) If ItemNum == 1 SourceContainer.RemoveItem(MiscItem01,1) ElseIf ItemNum == 2 SourceContainer.RemoveItem(MiscItem02,1) ElseIf ItemNum == 3 SourceContainer.RemoveItem(MiscItem03,1) EndIf EndFunction Link to comment Share on other sites More sharing options...
dylbill Posted March 27, 2020 Share Posted March 27, 2020 I think you can simplify it a bit. You could just use a function to check if the aggressor has the item, and to get another item if they don't. Also, I would use a formlist instead of individual items, because that way you can add or remove items easily later. I love my formlists. Something like This: Formlist Property MiscItems Auto Event OnHit(ObjectReference akAggressor, Form akSource, Projectile akProjectile, Bool abPowerAttack, Bool abSneakAttack, Bool abBashAttack, Bool abHitBlocked) Int Max = (MiscItems.GetSize() - 1) int random = Utility.RandomInt(0, Max) Form Item = MiscItems.GetAt(Random) If (akAggressor.GetItemCount(Item) > 0) akAggressor.RemoveItem(Item , 1) Else Item = FindMiscItem(akAggressor, Random, Max) If Item != none akAggressor.RemoveItem(Item , 1) Endif Endif EndEvent Form Function FindMiscItem(ObjectReference Ref, Int Index, Int Max) Int I = -1 While I < Max I += 1 If Index == Max Index = 0 Else Index += 1 EndIf Form ItemB = MiscItems.GetAt(Index) If Ref.GetItemCount(ItemB) > 0 Return ItemB Endif EndWhile EndFunction Link to comment Share on other sites More sharing options...
foamyesque Posted March 27, 2020 Share Posted March 27, 2020 I would go perhaps a little more sophisticated: Create a FormList with all the items you're intending to check, and a second, empty one. The first is your fixed reference; the second will act as an inventory tracker. Add an InventoryEventFilter to the NPC, passing in the first, full Formlist. This way, OnItemAdded() and OnItemRemoved() events will only fire (for your script) if they involve those particular items. If the NPC starts with the objects, copy, via script, all the items from the full FormList into the empty one. This is done because you can't remove via script things you didn't add via script, which is relevant later. Create an OnItemAdded() event. Since these will only fire for the items you're interested in because of the filter, you know it has to be one of those. Since you don't care which, nor of the amount, there's no need for further checks. Just use AddForm() to add it to the second, originally empty, FormList(). AddForm() will police a duplicate check for you, so you don't need to do anything else. Create an OnItemRemoved() event. Again, you don't need to check whether it's one of the items you're interested in, but you are interested in if there's any left, for which you can use GetItemCount(). If that returns 0 (or less), then use RemoveAddedForm() to clear it out of the tracking FormList. In your OnHit() event, get the Size() of the tracking FormList, If it is greater than 0, generate a random number between 0 and that size minus 1. Use GetAt() to retrieve the Form stored at that index, and then feed that into a RemoveItem() call. The upshot of all this is that there will be a tracking FormList that maintains a list of exactly which items are validly chooseable, and selects from them precisely once each time. If you want a failsafe, you can shift the OnHit() setup to use a loop to pre-emptively check if the item it's about to try to remove exists in the inventory, and if it does not, to remove it from the tracking FormList and try again until either it removes something from the NPC, or there's nothing left to try. Link to comment Share on other sites More sharing options...
NexusComa Posted March 27, 2020 Share Posted March 27, 2020 (edited) Different angle on it ... minimal cycle time /minimal stack. Scriptname CheckOnHitScript extends ActiveMagicEffectMiscObject Property MiscItem01 AutoMiscObject Property MiscItem02 AutoMiscObject Property MiscItem03 AutoInt random = 0;Event OnInit() GoToState("done")EndEvent;---------------------------Event OnHit(ObjectReference akAggressor, Form akSource, Projectile akProjectile, bool abPowerAttack, bool abSneakAttack, bool abBashAttack, bool abHitBlocked) If(akAggressor as Actor) If(akSource as Weapon) random = Utility.RandomInt(1, 3) If random == 1 If(akAggressor.GetItemCount(MiscItem01) > 0) akAggressor.RemoveItem(MiscItem01, 1) ElseIf(akAggressor.GetItemCount(MiscItem02) > 0) akAggressor.RemoveItem(MiscItem02, 1) ElseIf(akAggressor.GetItemCount(MiscItem03) > 0) akAggressor.RemoveItem(MiscItem03, 1) Endif ElseIf random == 2 If(akAggressor.GetItemCount(MiscItem02) > 0) akAggressor.RemoveItem(MiscItem02, 1) ElseIf(akAggressor.GetItemCount(MiscItem03) > 0) akAggressor.RemoveItem(MiscItem03, 1) ElseIf(akAggressor.GetItemCount(MiscItem01) > 0) akAggressor.RemoveItem(MiscItem01, 1) Endif ElseIf random == 3 If(akAggressor.GetItemCount(MiscItem03) > 0) akAggressor.RemoveItem(MiscItem03, 1) ElseIf(akAggressor.GetItemCount(MiscItem01) > 0) akAggressor.RemoveItem(MiscItem01, 1) ElseIf(akAggressor.GetItemCount(MiscItem02) > 0) akAggressor.RemoveItem(MiscItem02, 1) Endif EndIf EndIf EndIfEndEvent;State done ;do nothingEndState Edited March 27, 2020 by NexusComa Link to comment Share on other sites More sharing options...
ReDragon2013 Posted March 27, 2020 Share Posted March 27, 2020 (edited) TangerineDog wrote: "Now I just need to add a variable that differentiates between the first and the second run-through to avoid an item being checked for twice in one hit."The way to solve a mistake, is not to add more and more things like patch. If the first patch is not working, we take a second one, if this is not working, we .. Try to find the mistake (once) and use a proper code to avoid the mistake. Voila.. a patch isn't required anymore.I would suggest to use an array to predefine the miscobjects you want to remove. tdogCheckOnHitMGEFScript Scriptname tdogCheckOnHitMGEFScript extends ActiveMagicEffect ; https://forums.nexusmods.com/index.php?/topic/8531238-make-script-return-to-top/ ; We assume spell is an ability! MiscObject[] PROPERTY myList auto ; use CK to fill this array with miscobjects, you want to remove ; MiscObject Property MiscItem01 auto ; MiscObject Property MiscItem02 auto ; MiscObject Property MiscItem03 auto ; TangerineDog wrote: "I am adding a spell to the player that checks for certain items ; in an opponents inventory, when npc hits the player and removes one of them on each hit" ; -- EVENTs -- EVENT OnEffectStart(Actor akTarget, Actor akCaster) Debug.Trace(" OnEffectStart(" +myList.Length+ ") - target = " +akTarget+ ", caster = " +akCaster) ; debugging only IF (myList.Length > 0) gotoState("Waiting") ; ### STATE ### ENDIF ENDEVENT EVENT OnEffectFinish(Actor akTarget, Actor akCaster) gotoState("") ; ### STATE ### ENDEVENT ;============================== state Waiting ;============ EVENT OnHit(ObjectReference akAggressor, Form akSource, Projectile akProjectile, Bool b1, Bool b2, Bool b3, Bool b4) IF (akAggressor as Actor) ELSE RETURN ; - STOP - no actor, safety net ENDIF ;--------------------- IF (akSource as Weapon) ;|| (akProjectile) ELSE RETURN ; - STOP - do nothing for spell attack /or/ enchantment /or/ projectile ENDIF ;--------------------- gotoState("") ; ### STATE ### myF_Action(akAggressor) gotoState("Waiting") ; ### STATE ### ENDEVENT ;======= endState ; -- FUNCTION -- ;----------------------------------------------- FUNCTION myF_Action(ObjectReference akAggressor) ;----------------------------------------------- int i = Utility.RandomInt(0, myList.Length - 1) ; get random entry from array IF (akAggressor.GetItemCount(myList[i]) > 0) akAggressor.RemoveItem(myList[i], 1) ; remove a item (randomized array entry), if any ENDIF ENDFUNCTION Hints: Use a more suitable and unique scriptname. In nexus forum you can mark the whole papyrus source code and click on this button ""<>" to make them highlighted syntax. Edited March 27, 2020 by ReDragon2013 Link to comment Share on other sites More sharing options...
NexusComa Posted March 27, 2020 Share Posted March 27, 2020 If he is only using 3 things there is really no need to over code this. I did however in middle of the night realize it would need a check to see if it was actually from an enemy and hit (weapon) type. If that was to be only melee.He intends to put this script on the player (something I would never do). So this is a script that definitely need a sleep state when not in play. I really like the different approaches. Dylbill's formlist code is pure gold.ReDragon2013 recursive call and use of states is next level stuff. And as always IsharaMeradin code is a rock. Some of the talent here is more than a person could ever ask for. Link to comment Share on other sites More sharing options...
TangerineDog Posted March 27, 2020 Author Share Posted March 27, 2020 (edited) If he is only using 3 things there is really no need to over code this. I did however in middle of the night realize it would need a check to see if it was actually from an enemy and hit (weapon) type. If that was to be only melee.He intends to put this script on the player (something I would never do). So this is a script that definitely need a sleep state when not in play. I really like the different approaches. Dylbill's formlist code is pure gold.ReDragon2013 recursive call and use of states is next level stuff. And as always IsharaMeradin code is a rock. Some of the talent here is more than a person could ever ask for. I can only echo that - and to all who posted, thank you! FWIW, my own script is working fine so far (edit: and this is not to say that I'm not eagerly going through your scripts trying to learn how to improve my own scripts myself), it is however attached to an Ability I've given the Player.You say you wouldn't do that - could you explain why you wouldn't? Edited March 27, 2020 by TangerineDog Link to comment Share on other sites More sharing options...
NexusComa Posted March 27, 2020 Share Posted March 27, 2020 (edited) I tend to stay away from anything that is in constant interaction. I try to leave as small of a footprint as possible in every script. If you notice the script I posted here not only sits in the done /waiting state 99.99% of the time. But even when called will only hit a few of the IF statements and back out. It may look a bit over done but it's actually taking the shortest path possible to get the task done. With the lest amount of cycle time. This is an old habit for me as when I started programming it was late 80's. Back then every single cycle counted. I also started in assembler so I've learned stack matters. Today that isn't such a big deal ... unless you're dealing with a program like Skyrim that has 100's of scripts in queue at the same time. I really like Dylbill's code it is very generic (only truly skilled programmers write code in this fashion) but I would not use a code like that for something in constant interaction. I would use a code like that for a hit it once and done however. The concern would be if many enemy started hitting you at the same time. If the hits where right on top of each other. You will need a script that is super quick about what it is doing. Using as little cycle time as possible. As with any code the last thing you want is a crash or a constant memory dump to keep up. This will undoubtedly cause unforeseen odd problems if that starts happening. The kind of problem that can't even be debugged as the logic is sound. It's just trashing itself and other scripts with the constant calls. So for me it's best to just avoid that whole thing. If I was to do something like what you are doing it would only be in a area and not the entire game. To me my 1st concern is what the game is doing already. My code comes 2nd to that and it shows in my coding style. Again this may very well be old school babble. Computers today are so much more powerful and cycle time is almost endless. There is also an amount of consideration you have to give to other codes from other mods dealing with the same parts of the game you are. In short the less you push the stack the less chance of problems. So for me to attach a script to the player is pretty much off the table. I would look for a different way. If I was forced to do it. I would go right to a minimal style. The code above could be absolutely drilled and not fail as at most it will only take up 10 cpu cycles and hardly touch the stack. ReDragon2013 is also generic and really knows how to use states well. You can easily see the professional style in the way the code is laid out (everything clearly rem'ed out and in it's proper place). Also great foresight. As far as IsharaMeradin, I'm starting to think he is a Skyrim developer ... :yes: Edited March 27, 2020 by NexusComa Link to comment Share on other sites More sharing options...
IsharaMeradin Posted March 27, 2020 Share Posted March 27, 2020 ROFL!!! Not a Skyrim developer. Had to teach myself how to script in papyrus. The game has plenty of examples.... so I may have picked up a thing or two from the developers. Link to comment Share on other sites More sharing options...
Recommended Posts