Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing mana potion in stonehenge crypt chest #226

Merged
merged 1 commit into from May 8, 2021
Merged

Missing mana potion in stonehenge crypt chest #226

merged 1 commit into from May 8, 2021

Conversation

szapp
Copy link
Collaborator

@szapp szapp commented Apr 24, 2021

Describe the bug
Due to a typo, there's a mana potion missing in one of the chests in the crypt under the stonehenge.

Expected behavior
A mana potion is now correctly inserted in one of the chests in the crypt under the stonehenge.

Additional context
Bug and fix provided by N1kX94.

[% oCMobContainer:oCMobInter:oCMOB:zCVob 64513 16024]
pack=int:0
presetName=string:
bbox3DWS=rawFloat:-35454.0898 2280.677 -15369.6855 -35365.7734 2357.57202 -15269.3906
trafoOSToWSRot=raw:ceb5a63e00000000a40c723f000000000000803f00000000a40c72bf00000000ceb5a63e
trafoOSToWSPos=vec3:-35412.4219 2280.677 -15320.7129
vobName=string:CHEST
visual=string:CHESTBIG_OCCHESTMEDIUMLOCKED.MDS
showVisual=bool:1
visualCamAlign=enum:0
cdStatic=bool:0
cdDyn=bool:1
staticVob=bool:1
dynShadow=enum:0
[visual zCModel 0 16025]
[]
[ai % 0 0]
[]
focusName=string:CHEST
hitpoints=int:10
damage=int:0
moveable=bool:0
takeable=bool:0
focusOverride=bool:0
soundMaterial=enum:0
visualDestroyed=string:
owner=string:
ownerGuild=string:
isDestroyed=bool:0
stateNum=int:1
triggerTarget=string:
useWithItem=string:
conditionFunc=string:
onStateFunc=string:
rewind=bool:0
locked=bool:-1
keyInstance=string:ItKe_Focus5
pickLockStr=string:
contains=string:FOCUS_5,ITKELOCKPICK:2,ITFOBEER:2,ITAMARROW:45,ITMINUGGET:521,,ITFO_POTION_MANA_03
[]

@AmProsius AmProsius added the validation: required label Apr 4, 2021
@AmProsius
Copy link
Owner Author

@AmProsius AmProsius commented Apr 4, 2021

contains=string:FOCUS_5,ITKELOCKPICK:2,ITFOBEER:2,ITAMARROW:45,ITMINUGGET:521,,ITFO_POTION_MANA_03

changed to

contains=string:FOCUS_5,ITKELOCKPICK:2,ITFOBEER:2,ITAMARROW:45,ITMINUGGET:521,ITFO_POTION_MANA_03

@AmProsius AmProsius added this to VOB property in Fix templates Apr 4, 2021
@AmProsius AmProsius added compatibility: easy provided fix impl: change obj var type: world bug validation: validated and removed validation: required labels Apr 4, 2021
@szapp
Copy link
Collaborator

@szapp szapp commented Apr 5, 2021

This is an interesting issue. Typically this is just a VOB property assignment (as already mentioned), but I want to check what happens once the chest content is initialized. The thing to check: Once the chest content is initialized (i.e. chest opened), is the content stored as inventory list or is the "contains" string property updated on archiving (saving the game).

If the latter, we would have to watch out to not create an exploit, where the player can take the potion only, close the chest, save, load, and the potion is recreated.

@szapp szapp self-assigned this Apr 5, 2021
@szapp
Copy link
Collaborator

@szapp szapp commented Apr 18, 2021

Unfortunately, the contains property is only considered on first creation of the MOB. Afterwards, its content is stored explicitly. Therefore, altering that string will not fix the issue. Due to the possible exploit mentioned above, I propose to

  1. check if the potion item instance exists in the scripts using G1CP_IsItemInst or even getting the symbol index already for later with G1CP_GetItemInstId and then checking against -1 for validity
  2. find the chest by its position -35412.4219, 2280.677, -15320.7129 using G1CP_FindVobByPosF
  3. confirm its a oCMobContainer object with Hlp_Is_oCMobContainer
  4. assign it to a oCMobContainer variable, e.g. var oCMobContainer mob; mob = _^(vobPtr);
  5. confirm that its contains string is FOCUS_5,ITKELOCKPICK:2,ITFOBEER:2,ITAMARROW:45,ITMINUGGET:521,,ITFO_POTION_MANA_03 by using Hlp_StrCmp on mob.contains
  6. check if it's still locked (this is the important part), done like so
  7. check if the potion is already in the inventory, done like so
  8. and then add the missing potion (not to the contains string but to the MOB inventory), using an engine function like oCMobContainer::Insert

Since a chest cannot be locked again after it was opened once, this will serve as an indicator to prevent the exploit.

Reverting the fix will work analogously, but only under the condition that the G1CP fix was applied previously.

@szapp szapp added compatibility: difficult and removed compatibility: easy labels Apr 18, 2021
@szapp szapp added this to the v1.2.0 milestone Apr 22, 2021
@szapp szapp added this to To Do in v1.2.0 via automation Apr 22, 2021
@szapp szapp changed the title Missing mana potion in crypt chest Missing mana potion in stonehenge crypt chest Apr 24, 2021
@szapp szapp removed their assignment Apr 24, 2021
@szapp szapp moved this from To Do to In Progress in v1.2.0 Apr 24, 2021
@szapp
Copy link
Collaborator

@szapp szapp commented Apr 24, 2021

The test is manual: The key is placed in the PC's inventory and the potion should be in the chest. What may be worth checking is, whether the fix is correctly reverted when removing the patch if and only if the chest was not opened yet.

@AmProsius AmProsius merged commit 86a6d1f into master May 8, 2021
v1.2.0 automation moved this from In Progress to Done May 8, 2021
@AmProsius AmProsius deleted the bug226 branch May 8, 2021
szapp added a commit that referenced this issue Jan 18, 2022
Finding VOBs by their exact position can now be further narrowed down by
their class. Like before only the first matching VOB will be found. The
function can now also detect light VOBs which where previously ignored.
For details, see the header commment of ent of 'G1CP_FindVobByPosPtr'.
Common class definitions are defined as constants in constants.d. All
affected fixes are updated.

Refs #46 #51 #52 #127 #212 #213 #226
szapp added a commit that referenced this issue Jan 19, 2022
Instead of specifying a class to narrow down the search (see previous
commit), the option is now more versatile: Now the last parameter is a
callback function. It can either be one of the existing class check
functions or a custom function to narrow down the search more flexibly
and potentially based on more or more complex checks.

Refs #46 #51 #52 #127 #212 #213 #226
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compatibility: difficult impl: change obj var provided fix type: world bug validation: validated
Projects
Fix templates
Change VOB property
v1.2.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants