search

exochron / MountJournalEnhanced

This is an interface addon for World of Warcraft, which extends the ingame mount journal with some cool stuff.
https://www.curseforge.com/wow/addons/mount-journal-enhanced
GNU General Public License v3.0
13 stars 2 forks source link

Added functionality to create a note for each mount #61

Closed finnlux closed 1 year ago

finnlux commented 1 year ago

Functionality requested by a streamer. Thought it would be a good feature request for me to branch into LUA with.

exochron commented 1 year ago

Hey, thank you for this Pull Request. You are actually the first one, who adds a whole new feature here. 👍 I'm going to add this into the next version after cleaning and testing it a bit. 😃

Also, I'm a bit curious about that streamer. Do you have a link of her/him?

finnlux commented 1 year ago

Sweet. Let me know what sorts of things you clean up. I think it all works but there are probably things I've done "incorrect" because of not fully understanding LUA scoping or capabilities.

The streamer is TouchpadWarrior. He streams almost every night at 11pm CST. He has 879 mounts. https://www.twitch.tv/touchpadwarrior

exochron commented 1 year ago

now worries. scoping is the first hurdle to overcome as aspiring addon developer. 😉 by default all variables or functions are public/global. that means every other addon could access or overwrite them. (Similar to JavaScript with all variables within a document.) So it's just good practice to keep the public clutter as minimal as possible to avoid naming collisions with other addons.

exochron commented 1 year ago

okay, so far so good. :) I've moved some stuff around. For instance the note is displayed within the gametoolip of mounts instead of an extra tooltip. (I'd like to keep the UI style close to the original mount journal.) The note is also displayed underneath the lore text. You can check it out in branch 2.19.

finnlux commented 1 year ago

Oh I see you got rid of the tooltip altogether. Yeah I struggled on finding the right frame/window to make for the dialog and the tooltip both but what you did makes sense. I see I also missed a localization. Thanks for cleaning it up!