Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LibCustomNames support #4480

Merged
merged 1 commit into from
Jul 4, 2024
Merged

LibCustomNames support #4480

merged 1 commit into from
Jul 4, 2024

Conversation

mrbuds
Copy link
Contributor

@mrbuds mrbuds commented May 30, 2023

Add support for https://github.com/Jodsderechte/LibCustomNames
I think Jods addon needs a few reviews from addon authors before merging this to be sure interface isn't going to need too much changes

@mrbuds mrbuds added the 👩‍🔬 Work in Progress This pull request is still being actively developed and shouldn't be merged. label Jun 2, 2023
WeakAuras/Types.lua Outdated Show resolved Hide resolved
@@ -73,6 +73,17 @@ local LDBIcon = LibStub("LibDBIcon-1.0")
local LCG = LibStub("LibCustomGlow-1.0")
local LGF = LibStub("LibGetFrame-1.0")

local LibCustomNames = IsAddOnLoaded("LibCustomNames") and LibStub("LibCustomNames") -- optional addon
if LibCustomNames then
WeakAuras.GetName = LibCustomNames.Get
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i basically added the whole set of available UnitName functions incase they are needed in certain spaces https://github.com/Jodsderechte/CustomNames/blob/main/README.md

@InfusOnWoW
Copy link
Contributor

There's actually already a sort-of standard for custom names in the role playing addon community, apparently the common protocol is this: https://github.com/Total-RP/Total-RP-3/wiki/Mary-Sue-Protocol

I haven't looked to deep into it, the protocol is a bit suboptimal for our use cases and the whole functionality is obviously beyond what we need. But maybe we should investigate that a bit.

@Jodsderechte
Copy link
Contributor

I was trying to look into actual implementation of this but trying to find the actual doc for that had me running in circles. i eventually found https://github.com/wow-rp-addons/LibMSP which seems to be the most up to date version. However their wiki is just a parsed version of https://moonshyne.org/msp/ which didn't help me in the first place. However i feel like my database would explode trying to save all the names of the people you were randomly in group with/moused over. I'd also need to add options to turn off that nicknaming which kinda defeats the purpose of the "lib" (thats not a lib) to be very lightweight. It also seems like you can only ever have one addon active that is implementing libMSP or they create conflicts https://github.com/wow-rp-addons/LibMSP/wiki/Original-Mary-Sue-Protocol-documentation#addon-conflicts-and-how-to-avoid-them but i haven't checked that in detail. Maybe contacting those rp addon devs to implementing the saving of names in the databse would be the better bet.

@InfusOnWoW
Copy link
Contributor

As far as I undestood, there can be multiple addons requesting name information, but only one responding to name request information. Which is a pretty reasonable.

With the library/protocol you can request the nickname for a given player/realm but that's an async method, which is a bit problematic for WA's usage of unit formatting functions.

@Jodsderechte
Copy link
Contributor

Jodsderechte commented Jun 22, 2023

Also %name text replacements don't work yet unsure if that is something you want to support. Same for condition -> UnitName/Realm and Trigger -> UnitName/Realm (if unit exists ofc. Could also be added for combatlog triggers but you'd need to add Realm if you want to GetName) (either add both or none i guess). I also changed Lib.Get() to also allow for single name fetching (it will assume realm = GetPlayerRealm())

@mrbuds
Copy link
Contributor Author

mrbuds commented Jun 23, 2023

Also %name text replacements don't work yet unsure if that is something you want to support. Same for condition -> UnitName/Realm and Trigger -> UnitName/Realm (if unit exists ofc. Could also be added for combatlog triggers but you'd need to add Realm if you want to GetName) (either add both or none i guess). I also changed Lib.Get() to also allow for single name fetching (it will assume realm = GetPlayerRealm())

I think having only text formatters support, without touching aura's triggers & loading, is good enough at least for now
It also add helper functions to your addon for custom coded auras for complex needs

@Jodsderechte
Copy link
Contributor

Did some ingame testing it seemed to work without any issues on retail: do note i do not have classic or tbc installed so can't adhere to how it works there.

Regarding the MarySue protocol i talked to meo here about it and came to the conclusion it's not realy sth worth adding right now.

Adding it would provide 2 issues.

  1. since as infus already mentioned requesting data of MSP is async would require adding callbacks to the unit formater.
  2. the idea for CustomNames was to give the player controle over how other players appear on his screen. Using information others can set themselfes would result in a mess like with DetailsNicknaming which wasn't the goal.

That being said adding a plugin later on to CustomNames to allow it to fetch info of the MarySue protocol would easily be achievable and be the cleaner solution (as otherwise you'd need to atleast be able to toggle using that info anyways)

@Jodsderechte
Copy link
Contributor

CustomNames support has now been added to DBM. DeadlyBossMods/DeadlyBossMods#1125
This means there is now support for Dbm,MRT, ELvUI , Details and Plater aswell as someone created a grid plugin

@mrbuds mrbuds force-pushed the smallfix branch 3 times, most recently from 8d5fe92 to 8de88cc Compare July 2, 2024 19:58
Copy link
Contributor

@Stanzilla Stanzilla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DIDN'T TEST IN GAME BUT CODE LGTM!

@mrbuds mrbuds added 🆕 Feature Preview This is a draft intended to show a preview of an upcoming feature. and removed 👩‍🔬 Work in Progress This pull request is still being actively developed and shouldn't be merged. labels Jul 2, 2024
Copy link
Contributor

github-actions bot commented Jul 2, 2024

An experimental build of WeakAuras with the changes in this pull request is available here.
Build Time: Tue Jul 2 20:09:40 UTC 2024

@Jodsderechte
Copy link
Contributor

From a quick test on beta seems to still work correctly. However i didn't do a more advanced check only sitting around in OG as i currently don't have a sub.

@InfusOnWoW InfusOnWoW merged commit 61996c3 into WeakAuras:main Jul 4, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🆕 Feature Preview This is a draft intended to show a preview of an upcoming feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants