ToME: the Tales of Maj'Eyal

Everything about ToME
It is currently Sat Jul 21, 2018 3:19 pm

All times are UTC




Post new topic Reply to topic  [ 2 posts ] 
Author Message
PostPosted: Thu Apr 05, 2018 2:25 pm 
Offline
Higher

Joined: Sun Dec 10, 2017 12:27 am
Posts: 78
I have been pestering Shibari about a bug I uncovered while working on a class addon, and think I have enough to report properly now.

Sometimes, the __project_source and resolveSource architecture fails to result in delayedLogDamage aggregating lines correctly.

An example of the behaviour can be reproduced in vanilla ToME by damaging yourself while affected by Reality Smearing. Instead of the expected line:
Code:
Player hits Player for (45 converted), 106 arcane (106 total damage)
there are two separate entries in the log, which are not necessarily adjacent:
Code:
Player hits Player for 106 arcane damage
Reality Smearing hits player for (45 converted) damage


I thought this was probably restricted to the callback architecture, and have traced the specific behaviour in my addon, and in vanilla with reality smearing, to the replacement of the existing __project_source in fireTalentCheck (/modules/tome/class/Actor.lua#5328‑5341 in the current repository). This results in the called-back sustain being treated as the source when it should still resolve to the self in the case of damage mitigation from self-damage.


Top
 Profile  
 
PostPosted: Wed Apr 11, 2018 9:01 pm 
Offline
Higher

Joined: Sun Dec 10, 2017 12:27 am
Posts: 78
I hacked together a poor solution, which I threw at Shibari, but think there is a small tweak required compared to the merge request he posted:
Code:
function _M:fireTalentCheck(event, ...)
    if self[store] then upgradeStore(self[store], store) end
    if self[store] and next(self[store].__priorities) then
       local old_ps = self.__project_source
-- Changes to code here
      local src_check, keep_old -- we need these to be available outside the if
      if event == "callbackOnHit" then
         src_check = (select(2,...)) -- if this is a callbackonHit, the second parameter is the source of the hit, I think this is fairly awful coding practice, but I am not super-familiar with Lua
         keep_old = (src_check.resolveSource and src_check:resolveSource() or src_check) == self --we shouldn't overwrite our __project_source if we're the source of the damage (we are more concerned with why we hurt ourselves than what the callback is)
-- original hackfix had this line:
--         old_ps = old_ps or self
-- This leaves us with an instantiated __project_source at the end of the function call, though, even if it wasn't at the start, so it may be better to leave this as nil and accept more logic overhead in the ternaries
      end
       for _, info in ipairs(self[store].__sorted) do
          local priority, kind, stringId, tid = unpack(info)
          if kind == "effect" then
            self.__project_source = keep_old and (old_ps or self) or self.tmp[tid] -- change from MR, to account for old_ps == nil
             ret = self:callEffect(tid, event, ...) or ret
          elseif kind == "object" then
            self.__project_source = keep_old and (old_ps or self) or tid -- change from MR, to account for old_ps == nil
             ret = tid:check(event, self, ...) or ret
          else
            self.__project_source = keep_old and (old_ps or self) or self.sustain_talents[tid] -- change from MR, to account for old_ps == nil
             ret = self:callTalent(tid, event, ...) or ret
          end
          self.__project_source = old_ps
       end
    end
    return ret
end

This fixes the bug I encountered, but isn't perfect. A similar change might need to be made to _M:iterCallbacks so that the same tests are applied inside the wrapper functions. In this case, it would be essential that the more complex ternary be used, to avoid messing up the __project_source as you iterate over the callbacks.

Potential bugs exist in any other callback where there are both a source and a target entity passed and where both could resolve to the same source (callbackOnDealDamage, callbackOnKill, callbackOnSummonDeath) but only callbackOnDealDamage is likely to actually result in anything odd being printed to the log.


Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 2 posts ] 

All times are UTC


Who is online

Users browsing this forum: Baidu [Spider] and 4 guests


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Search for:
Jump to:  
Powered by phpBB® Forum Software © phpBB Group