brittyazel / Neuron

A full-featured World of Warcraft action bar and interface replacement addon
https://www.curseforge.com/wow/addons/neuron
MIT License
31 stars 21 forks source link

fishing state #51

Open pjotrpottervogel opened 6 years ago

pjotrpottervogel commented 6 years ago

hello,

the fishing state needs another localization because " fishing pole" is the english term. so [worn:fishing pole] (Neuron_States.lua 91) should be [worn: angelruten] in german client.

thx for reading and developing further - i really missed macaroon a long time and now it seems neuron is getting all the old functionality back.

brittyazel commented 6 years ago

Thanks for the heads up, I'll fix this asap :-)

Slowly but surely I'm bring the codebase back to life. It didn't age well unfortunately, but it's getting better week after week. If I was just coding the addon from scratch it'd probably be easier, but working on a live project has it's own sets of challenges. I can't just make the changes that need to be made sometimes, as it will break everyone's configs :-( Hence I have to be very slow and methodological about everything I do

pjotrpottervogel commented 6 years ago

sure, i get that - you do well - thx again ^^

brittyazel commented 6 years ago

What is the result of this big? Do you see the English string or is the fishing pole visibility modifier not working?

pjotrpottervogel commented 6 years ago

the fishing pole visibility does not work, the macro condition is hardcoded with the language of the client ^^

brittyazel commented 6 years ago

Ugh ok. I didn't know that, so I'll have to figure out if I can query the game for the translated string name

brittyazel commented 6 years ago

Can you try for me setting it to "fishing poles" instead of "fishing pole" in the NeuronStates file? It should be in both the managed_action_states and managed_bar_states tables.

Apparently a while back they changed the macro conditional to be "fishing poles" to differentiate it from "fishing pole" which is the specific item. This might be language-inspecific

pjotrpottervogel commented 6 years ago

sorry, no - i tried now but it does not work.

pjotrpottervogel commented 6 years ago

but it should be in english client fishing poles anyway because that is the category of the item - like swords, daggers,... and that would be schwerter, dolche in the german client ^^ those language specific category macro conditions were always like that, don't know why - maybe a translator wanted to make an extra good job XD

brittyazel commented 6 years ago

I'm confused how any of the other states are working in other languages then... Do you know if there is a list of the foreign language conditionals

pjotrpottervogel commented 6 years ago

only equipment i think

pjotrpottervogel commented 6 years ago

so if you get the localized catagory of the fishing poles from a function it should be ok

pjotrpottervogel commented 6 years ago

this should deliver the localized fishingpoles local , , , , , , fishingpoles, , , , , , , , , , = GetItemInfo(6256)

ylixir commented 1 year ago

i think we can actually solve this in a generic way with GetItemSubclassInfo. although i don't know how relevant this is since i don't think we even need to equip fishing poles anymore, there's the new profession equip thing, and we have the new interact button

references https://wowpedia.fandom.com/wiki/API_GetItemSubClassInfo https://wowpedia.fandom.com/wiki/ItemType

AlexFolland commented 1 year ago

I solved this in my addon like this:

function IsMainHandAFishingPole()
    local mainHandID = GetInventoryItemID("player", INVSLOT_MAINHAND)
    if mainHandID then
        local itemClassID, itemSubClassID = select(6, GetItemInfoInstant(GetInventoryItemID("player", INVSLOT_MAINHAND)))
        return ((itemClassID == Enum.ItemClass.Weapon) and (itemSubClassID == Enum.ItemWeaponSubclass.Fishingpole))
    end
end

Feel free to copy this function and change it as desired. It's pretty generic and uses the most modern Blizzard-provided functions for determining item types.

ylixir commented 1 year ago

Since fishing has been completely revamped in dragonflight, my inclination is to remove this feature altogether. I don't want to leave a bad experience for classic, and I don't want to work a feature that I can't test.

There is some code in there for custom states. I don't understand it, and it has no UI. Getting something like that working seems like a much better approach here. Classic users can have their cake through a custom event, and we don't have to eat support for a classic only feature

AlexFolland commented 1 year ago

Why not at least use my function to check for fishing state in Wrath Classic? I can guarantee it works, as it works well in my addon which is maintained and tested in Wrath Classic.

ylixir commented 1 year ago

several reasons: 1) your code doesn't help with the systemic issue of localization which is the underlying problem 2) your code is not a macro conditional and so it is not a drop in solution. probably isn't a solution at all tbh. idk because: 3) as i said above, i have no interest in introducing new features for classic 4) custom states solves all of these problems

AlexFolland commented 1 year ago
  1. your code doesn't help with the systemic issue of localization which is the underlying problem

This is incorrect. Checking the item class ID and item subclass ID avoids the problem of localization. No strings are used; only enums which are not locale-specific.

  1. your code is not a macro conditional and so it is not a drop in solution. probably isn't a solution at all tbh. idk because:
  2. as i said above, i have no interest in introducing new features for classic
  3. custom states solves all of these problems

I don't know what the rest of your points mean in the context of Neuron, but just wanted to respond to your first point. Thanks for your answer.

ylixir commented 1 year ago

I should have been more clear about what I meant about solving the localization problem. While that code does solve this one particular instance for this one particular type of equipment, it's not a general purpose systemic approach to the underlying issue of localized equipment names. We would have to make a separate function for every issue that was opened around localization differences in macros. So it's probably not scaleable. In my opinion, we need a way to deal with the problem in a general purpose way not just for this specific equipment type.

In the context of neuron for the rest of the comment: Neuron bar states in combat are managed through macro conditionals basically. The bars can't just run arbitrary code. They have to be set up ahead of time. If they need to change something in combat, the that change is controlled with macro conditionals. Changing that structure would require way more work than I'm going to invest in super niche feature that only a small amount of classic users would even care about.

With leads to the custom state idea. Theoretically we could let the users enter the conditionals for when to show certain bar states manually. Then we don't even have to care about localization. The user can just enter their German macro or whatever. We have several openissues asking for various states as well. It would be nice to just put those in a wiki or something instead of being hard coded into the codebase and requiring a new release etc.

Bonus, the custom states would work for classic users with zero extra effort from me. Which is basically the amount of work I'm willing to commit to for a classic feature ;-)