darfink / ItemAutocomplete

Chat autocomplete for item links in WoW Classic
https://curseforge.com/wow/addons/itemautocomplete
MIT License
4 stars 5 forks source link

Error on autocomplete selection using enter button #1

Closed MatthewBerkvens closed 3 years ago

MatthewBerkvens commented 4 years ago

Reproduction steps:

Other installed addons Too lazy to type them up, have a gyf https://giant.gfycat.com/CalmFailingAntbear.webm

Screenshots First time the error occurred:

Consecutive errors look like this:

darfink commented 4 years ago

Thanks for your report!

That is a lot of addons, and I'd probably end up with different versions if I downloaded them all. Is there any possibility you could ZIP your addons folder and upload at Firefox Send or file.io for me to investigate?

MatthewBerkvens commented 4 years ago

I update my addons several times a day, but I have some which aren't on the twitch app.

Here you go: https://gofile.io/?c=kGZZns

darfink commented 4 years ago

Thanks!

I've investigated and as I suspected; BugGrabber is the cause (an optional dependency of both Decursive & AdiBags). BugGrabber overwrites the seterrorhandler function to a no-op leaving other addons with no alternative.

ItemAutocomplete utilizes this function to set a temporary error handler. I personally believe it's very intrusive of BugGrabber to overwrite this API function, and as of now I cannot come up with any workaround.

2072 commented 4 years ago

Hi,

You should use Buggrabber callback system.

In Decursive's Dcr_DIAG.lua, take a look at the following functions:

MatthewBerkvens commented 3 years ago

@darfink I managed to fix this issue by cobbling together a different solution

in ChatAutocompleteIntegrator:Enable

self.KeyManager = CreateFrame("Frame", nil, UIParent)
self.KeyManager:SetFrameStrata("FULLSCREEN")
self.KeyManager:EnableKeyboard(true)
self.KeyManager:SetPropagateKeyboardInput(true)
self.KeyManager.Parent = self
self.KeyManager:SetScript("OnKeyDown", self.methods._OnMyIntercept)

New function

function ChatAutocompleteIntegrator:_OnMyIntercept(self, key)
  if self.Parent.buttonMenu and self.Parent.buttonMenu:IsShown() and key == "ENTER" then
    local editBox = GetCurrentKeyBoardFocus()
    self.Parent:_OnItemSelected(editBox, self.Parent.buttonMenu:GetSelection())
    self:SetPropagateKeyboardInput(false)
  else
    self:SetPropagateKeyboardInput(true)
  end
end

and commenting out everything except the return statement in ChatAutocompleteIntegrator:_HookChatMessageBeforeSend

It's not the cleanest solution, but it worked and might give you some inspiration on how to better fix this issue https://i.imgur.com/w5nE7Up.png

darfink commented 3 years ago

That's great input! This alternative solution seems (at first glance) much better than the current one. If I may ask, did you implement this for TBC Classic, or did you try this implementation in Vanilla Classic?

Regardless, the current Enter detection is a bit of a hack, and I have always wanted to replace it.

MatthewBerkvens commented 3 years ago

I made these edits during the TBC prepatch and it's currently working flawlessly, I haven't tried it in Vanilla Classic but I highly doubt it won't work there.

darfink commented 3 years ago

I've implemented a solution based on your suggestion, and it is working better than expected! I will test it out a bit more before releasing, but it's a much better solution so I'll try to have it ready ASAP.

Thanks a ton for the suggestion!

darfink commented 3 years ago

I did some additional adjustments: avoiding intercepting keys at all time, and preventing runtime taint (could occur when '/'-commands were used), but all in all, the solution seems great.

And since it was part of the v2.0.1 release, I now consider this issue resolved. https://www.curseforge.com/wow/addons/itemautocomplete