Skip to content

(BG1) Spells that create magical items have issues when concluding w Opcodes 111,122

BhryaenBhryaen Member Posts: 2,874
edited August 2012 in Fixed
This bugfix was proposed by some obscure BGEE forumer (and Beta Tester) named @Galactygon (*nudge, nudge, wink, wink*)

I'm not able to articulate it better, so I'll direct quote from @Galactygon:


OBSERVED:

Opcodes 111 (create magical weapon) and 122 (create inventory item) with durations create items and then remove them by internally applying opcodes 122 (remove item) and 123 (remove inventory item). This way of handling things has the following issues:
1. You may not refresh spells that create items because opcode 122 will be applied prematurely from the previous casting. For example, if you cast Flame Blade that lasts for 10 rounds and you try to refresh the spell after 8 rounds, the new casting will only last for 2 rounds because the .itm will be removed by virtue of the previous casting
2. In case of opcode 122 you may give the .itm to another .cre or drop it on the ground or in a container and the .itm will not disappear because at expiry it will attempt to remove the .itm from the .cre on which it was created
3. There is this default sound (EFF_M02) that is hardcoded, which would be nice to softcode or eliminate somehow
4. The created magical items are not dispellable (but this is fixed in ToBEx)

EXPECTED:
Opcode 255 (create inventory item (days)) on the other hand removes the item by applying an internal timer to the .itm which is stored on the inventory of the .cre. This is a much better way to to handle magically created items, and would fix all the issues if this method would be used by opcodes 111 and 122.

I would even go as far to suggest an alternative to the current ToBEx implementation to use the extra space at 0x10 and 0x11 of inventory items header of the .cre for a "dispellable" flag and bytes at 0x12 and 0x13 to store the casting level of the caster who created the item.

-Galactygon
Post edited by Bhryaen on
AndreaColomboIgneous

Comments

  • GalactygonGalactygon Member, Developer Posts: 412
    edited August 2012
    Opcodes 111 (create magical weapon) and 122 (create inventory item) with durations create items and then remove them by internally applying opcodes 122 (remove item) and 123 (remove inventory item). This way of handling things has the following issues:
    1. You may not refresh spells that create items because opcode 122 will be applied prematurely from the previous casting. For example, if you cast Flame Blade that lasts for 10 rounds and you try to refresh the spell after 8 rounds, the new casting will only last for 2 rounds because the .itm will be removed by virtue of the previous casting
    2. In case of opcode 122 you may give the .itm to another .cre or drop it on the ground or in a container and the .itm will not disappear because at expiry it will attempt to remove the .itm from the .cre on which it was created
    3. There is this default sound (EFF_M02) that is hardcoded, which would be nice to softcode or eliminate somehow
    4. The created magical items are not dispellable (but this is fixed in ToBEx)

    Opcode 255 (create inventory item (days)) on the other hand removes the item by applying an internal timer to the .itm which is stored on the inventory of the .cre. This is a much better way to to handle magically created items, and would fix all the issues if this method would be used by opcodes 111 and 122.

    I would even go as far to suggest an alternative to the current ToBEx implementation to use the extra space at 0x10 and 0x11 of inventory items header of the .cre for a "dispellable" flag and bytes at 0x12 and 0x13 to store the casting level of the caster who created the item.

    -Galactygon
    Post edited by Bhryaen on
  • GalactygonGalactygon Member, Developer Posts: 412
    Thanks for posting this Bhryaen!

    I'd like to elaborate on the last part of my suggested solution. There is an inconsistency between game time (kept track as a long) and the expiry values in the itm header of the .cre and .are formats (stored as two bytes at 0x08 and 0x09).

    I propose that the absolute game time value for item expiry is stored as a long that "jumps" from 0x08, 0x09 to 0x12, 0x13. The caster level (normally stored as a short) is then truncated to a single byte at 0x11 since it's unlikely to go above 255, and the item flags are stored at 0x10.

    This isn't a clean solution, but this doesn't change the size of the file format and should prevent existing mods from breaking.

    -Galactygon
    AndreaColomboCuvIgneousBhryaen
  • SethDavisSethDavis Member Posts: 1,812
    Can anyone give a specific scenario to test this? I have altered the code so that rather than triggering a remove item effect after a certain time, that time is set as the items duration.
  • BhryaenBhryaen Member Posts: 2,874
    @SethDavis
    That sounds about right, but I suppose I wouldn't know. There is an example in @Galactygon's quote above though regarding the spell Flame Blade and whether you can extend its duration by re-casting the spell (apparently you can't). I'm not sure what spell-created limited-duration ITMs are supposed to auto-delete which you can also drop before expiration, though maybe it can be spawned?
  • GalactygonGalactygon Member, Developer Posts: 412
    While I don't have the game in front of me right now, it should be testable by a short-duration magical item such as harm or any of the cause wounds. If you expended these items in vanilla right after the spell was cast and then tried to cast the spell again less than 6 seconds before the previous spell was completed, you will find out that the newly created created magical items disappeared on their own because of an embedded remove item.

    -Galactygon
    AndreaColombo
  • Avenger_teambgAvenger_teambg Member, Developer Posts: 5,862
    SethDavis said:

    Can anyone give a specific scenario to test this? I have altered the code so that rather than triggering a remove item effect after a certain time, that time is set as the items duration.

    The easiest created item is goodberry (jaheira knows it). But it might be permanent.
    Shillelagh and flame blade are also known by her.
    Just memorize 2 flame blades. Cast one, then cast another midway expiration.
    If the item is refreshed - good. Then just wait till the end of expiration.
  • BhryaenBhryaen Member Posts: 2,874
    @Galactygon
    So spells like Chill Touch, Vampiric Touch, Shocking Grasp all rely on creating temporary items? Is that true also for Polymorph that adds squirrel paws? In those cases though there's nothing the player could remove from inventory to test for the other element of duration expiring despite the created ITM not remaining in the caster's inventory.

    Oh where's @Igneous who knows the spell angles so well?
  • GalactygonGalactygon Member, Developer Posts: 412
    Chill Touch and Shocking Grasp create temporary .itms, while Vampiric Touch is just a short range spell like Maze or Imprisonment. Goes to show the original developers were not consistent with the implementation of touch spells.

    -Galactygon
    AndreaColomboBhryaen
  • SethDavisSethDavis Member Posts: 1,812
    Well, I hope that the m_wear variable wasn't being used by anything else cause I totally hijacked it.

    Potentially fixed - Summoned weapons and items now die at the appropriate times without using a timed destroy effect. This allows weapons to be refreshed.
    BhryaenGalactygonAndreaColombo
  • Avenger_teambgAvenger_teambg Member, Developer Posts: 5,862
    edited July 2012
    Heh, interesting. I thought m_wear was already used for this purpose. I think create item for days effect (0xff) even uses it.
  • SethDavisSethDavis Member Posts: 1,812
    edited July 2012
    @Avenger_teambg - It does set it, but there was actually no code anywhere that did anything with it. As far as I can tell the create_item_days effect was pretty much create item permanent

    And come to think of it, I've probably just turned it into create item so i can see what it looks like before it goes poof....
    AndreaColombo
  • TanthalasTanthalas Member Posts: 6,738
    edited August 2012
    Confirmed Fixed

    Tested this in three ways:

    1. As a level 6 Druid I casted Flame Blade and then waited 24 seconds before casting it again. The second Flame Blade lasted 42 seconds, consistent with the description of the spell.

    2. As a level 6 Druid I casted Flame Blade, waited 10 seconds, casted a successful Dispel that dispelled Flame Blade, and then recast Flame Blade. The second Flame Blade lasted 42 seconds, consistent with the description of the spell.

    3. Used Cause Serious Wounds and observed that the spell lasted for 15 seconds. Used a Cleric to cast Cause Serious Wounds, used it immediately on a target and then immediately recasted Cause Serious Wounds and it lasted 15 seconds.
    Bhryaen
  • Avenger_teambgAvenger_teambg Member, Developer Posts: 5,862
    edited August 2012
    That's impressive. Both the fix working and your testing. So, the item removal was not needed, and m_wear is useful for short times too. If you could drop the item (i doubt you can), you could watch it vanish from the ground too.
    [edit]
    Actually no, i changed goodberries to use duration. They vanish from inventory properly, but once dropped and taken, m_wear vanishes. I will bug that separately.
Sign In or Register to comment.