60 replies to this topic

[Ascension64]

OK, so I'm looking at 8AF65D BOOL CCreatureObject::RefreshStats(), which appears to be the same in SoA22941, SoA23037, and ToB26498. I'm judging that the visual effects are handled somewhat differently in 22941 and so the graphic doesn't get purged. But I can't debug 22941 because I don't have a valid 22941 install. Maybe you can help out there.

[Suslik]

Sure I can send you my 22941(?) executable. I am not sure where are you checking its version, but this is the one where the graphic is not purged after spawning: http://dl.dropbox.co...5148/BGMain.zip

I think it's ok just to replace exe's without doing any modifications to resources(at least for testing).

[Ascension64]

Oh no that's OK. I have had to manually install SoA so I had a proper installation of it.
But the answer is dumb obvious from what we already know.

In 22941, visual effect X has the cre as eOwner but is never referenced by the cre. It plays to completion and purges.

In 23037/26498, the extra code in CVisualEffect::AIUpdate() purges visual effect X because the cre already has an eVisualEffect reference. The extra code would also set the cre eVisualEffect if the cre did not already have one. But since it already does, the X is purged. Throughout this process, X is still never referenced by the cre. If we decide to add X's vid cell, X-dash, to the existing eVisualEffect, we effectively make the cre reference X-dash, causing it to undergo a variety of checks that it has not been done before.

I presume that the extra code added to CVisualEffect::AIUpdate() tightens up creature visual effects to prevent multiple visual effects being applied to the creature, which may cause a memory leak. I am not sure though, and I don't think removing the extra code will help if it was purposely added in the first instance.

[Suslik]

I agree that eliminating the code added in 23037 is plainly wrong, because it was probably added to prevent some undefined behavior/leaks.

But if creature already has a visual effect, can't we just add our new one as its child?

[Galactygon]

Doesn't replacing SPSTURNI.bam with a blank one fix this? Is this fix worth all that trouble?

-Galactygon

[Suslik]

Doesn't replacing SPSTURNI.bam with a blank one fix this?

Fix what? We are trying to repair the effect, not to eliminate it.

[Galactygon]

I have a hard time following this thread. I've picked up on 2 things that ought to be fixed:
- Spell Turning doesn't always reflect as it's supposed to
- A silver disk animation appears at the moment the effects are turned

-Galactygon

[Suslik]

Umm.. First problem is actually solved. See solutions above involving zeroing allocated memory. Second problem is that the disk animation does NOT appear whenever a spell is reflected. Only the first frame is showed(instead of whole animation).

[Ascension64]

I'll add to the second problem that most players probably never ever saw the disk animation because it only could be visualised in Shadows of Amn versions smaller than 23037. Some new code added somewhere between builds 22941 and 23037 caused the disk animation not to be played properly, and this was unintentional, likely to fix other issues with the engine.

At the moment, I am having difficulties 'restoring' this disk animation because of this extra code.

But if creature already has a visual effect, can't we just add our new one as its child?

As I mentioned, the existing creature's visual effect is highly effect-dependent and its vid cells are purged every time the creature is refreshed. Because the disk animation should is a one-off animation that isn't held by any effect, I cannot simply add the vid cell as a child of the visual effect. I will probably need a specific bypass of the extra code to allow this disk animation only to play to completion... unless we are all happy with the disk animation staying stationary.

Update: by looking at how the engine handles visual effects added to creatures, I've managed to work around the extra code by adding an extra condition that will skip the extra code check if the visual effect isn't flagged to be owned. After reviewing all the code that refers to visual effects, those added to creatures are mainly through CreateVisualEffect() [if added to the cre, the owned flag is set] and AddVisualEffect() [if cre already has a visual effect, a vid cell is added; if not, a visual effect is created with owned flag set]. When SPSTURNI is created, the owned flag isn't set. There doesn't seem to be any other visual effects that are created and directly added to creatures like the disk animation. Therefore, the extra code check will be skipped just for the disk animation, but not any of the other visual effects.

loc_654C61:
  if (pCre->GetType() == CGAMEOBJECT_TYPE_CREATURE) {
    if (pVisualEffect->m_dwFlags & CVISUALEFFECTFLAG_OWNED) { //added
      if (pCre->e == ENUM_INVALID_INDEX) {
        pCre->eVisualEffect = pVisualEffect->e;
      } else {
        if (pCre->eVisualEffect != pVisualEffect->e) {
          FreeObjectShare(pVisualEffect->eOwner, THREAD_ASYNCH, INFINITE);
          pVisualEffect->Purge();
        }
      }
    }
  }

Edited by Ascension64, 17 January 2012 - 12:35 AM.

[Suslik]

Because the disk animation should is a one-off animation that isn't held by any effect, I cannot simply add the vid cell as a child of the visual effect.

due to language barrier i am having really hard time understanding this sentence. Ok, but I have understood that we cannot simply add the vid cell to existing effect due to hard-coded issues which can be bypassed only by extra code.

unless we are all happy with the disk animation staying stationary.

Heh, lol. I suggest you should pay attention to this problem if and only if you feel like it. Personally I like the animation very much, I am glad that we've fixed it even if it remains stationary(I am currently fighting beholders and it looks pretty awesome). Though it would be better if it moved with the caster, dont overdo it - if you find any further work(attaching the effect to creature) unreasonable, of course you should switch to something more important.

[Suslik]

When SPSTURNI is created, the owned flag isn't set.

I see. Quite a weird idea to attach effect to a creature without setting flag "owned"(addressed to developers).

I suggest that's a good fix you've made, and hope that it will affect only spsturni and no other effects. Have you added the fix to TobEx? Can we test it?

[Dakk]

If I understood Ascenscion64's last edit, he's now actually made the animation move with the creature? That's dedication!

[Suslik]

Of course he is awesome, did you doubt that? Now waiting for him to tell us whether the fix will be added to TobEx or not.

[Ascension64]

Have a play
http://www.shsforums...post__p__532208

[Suslik]

I currently have TobEx v0.20. I have copied TobExLoader.exe/TobEx.dll to root BG directory and placed new configs to TobEx_ini. When I run the game via launcher I get:

TobEx: Throne of Bhaal Extender build 0.23.1.17 (Wed 18 Jan 2012 15:52:20)
InitUserPatches(): unable to open . Error code 2.
InitUserPatches(): FindClose() failed (error code 2)

And it seems like patches are not applied.

I have tried to look at TobEx0020.tpa incremental patch, but it seems only to modify stats.ids, trigger.ids and action.ids which are most likely irrelevant. And it also externalizes something called "mage spell hiding", but I have failed to understand what this code does:

//Externalise Mage Spell Hiding
COPY_EXISTING "hidespl.2da" "override/hidespl.2da"
  REPLACE_TEXTUALLY "VALUE" "UNUSED"
  
  COUNT_2DA_ROWS 2 nRows
  FOR (i = 1; i < %nRows%; i += 1) BEGIN
    SET_2DA_ENTRY %i% 1 2 "****"
  END
  SET_2DA_ENTRY 1 0 1 "****"
BUT_ONLY

how can I reproduce it manually and is it the way to fix that "unable to open" error?

//Update: ok, my bad. It seems I have forgotten to rename default_core.ini to TobExCore.ini. Patches Are applied now. But I still get the message "unable to open", though the game starts and everything seems to work fine.

Great job, asc64!

Edited by Suslik, 18 January 2012 - 04:11 AM.

[Ascension64]

I warned someone else about warning messages when manually upgrading.

[Suslik]

Okay, I'm reinstalling BWP anyway.

[Beleg33]

Just made an install with the latest tobex 0023. Fix seems to be working perfectly, great job!

[Pol]

Looks good.

I'm interested to know if the opcode #197 (Physical Mirror) can be fixed in the same way, it will sometimes start absorbing projectiles rather than reflecting them. Of course, it's less important than the work on Spell Shield as #197 isn't supposed to deflect a fixed number of projectiles and the functionality is preserved even when this bug pops up.

[Beleg33]

I actually tested physical mirror too on crazy tactics Illasera with her dispel arrows of doom and didn't see any absorbed arrow, all reflected.