04-05-11, 02:01 PM | #1 | |
Totems element - looking for feedback
I found out a little bit ago that shamans were not the only class to use the default TotemFrame element, and I've been meaning to update oUF_boring_totembar to not be shaman-only. I was also thinking that since DKs and druids use it too, maybe it's not really a niche feature anymore and that maybe haste would be interested in supporting it in oUF.
So I rewrote my addon with two things in mind: one, to copy the hell out of the style of the default oUF elements, and two, to only provide the features of the default Blizzard UI while allowing the user to override whatever they want. I am posting this here to see if haste is interested in adopting my code (or if I should just release this as my own plugin), and to get feedback from other authors about what can be improved. The plugin is attached to this post, and here is the (somewhat rambling) info text that I wrote along side it.
|
||
04-06-11, 03:41 AM | #2 |
Hey,
I can't really quickly glare at your code from my phone, but I can answer some of the questions you have. Most of this post is written in chunks, so there might be some odd rambling here and there :P. Some minor comments: SetupTotem: Based on your information, SetupTotem will replace user defined frames. In general oUF elements don't care what you give them, as long as the correct methods are there. So having the element replace pre-defined frames because they don't match a given type sounds like a bad idea. I'm not really sure if it should define a Icon and Cooldown frame for you if it isn't present. In the future I don't want the elements to automatically create a bunch of frames, textures and fontstring without the layout asking for it, but I also don't want it to be a major hassle to add elements to a layout. For now I would suggest to leave it as is, but don't replace already defined frames and such just because they aren't they type you want them to be. Adding sane On{Click,Enter,Leave} is fine btw. .map: To me, this sounds like a needless complication. Zariel did the same thing in the RuneBar element before, and it really is easier to manage this in the layout. The positioning doesn't need to match the order they are assigned to the .Totem table. Your questions: .ForceUpdate: Used to fake a true event to force the element to update. Mainly there to let people force oUF to update settings if something was changed by the user. Path: Previously oUF would register Overrides directly. This made it possible to disable a Override, without having the event unregister correctly, which means you end up with the element still being updated at times. The solution is to proxy event registrations through a common function, which then calls the correct end point without having to know if it has been changed. Disabling the default totem frame: It depends. Will the default totem element show, continue to update or do anything odd if you don't? Override and callbacks: Still can't open zip files, so I'm not entirely sure. Override sounds correct at least. I provide element:PreUpdate() and element:PostUpdate() "where it makes sense" and "where alternative solutions like using OnShow/OnLeave doesn't make sense". How this fits with the totem element I can't really answer. I'm also a fan of the "implement the bare base and see what people want" feedback loop :P. (I'll be able to take a proper look in 8 hours or so)
__________________
「貴方は1人じゃないよ」 |
|
04-06-11, 01:31 PM | #3 |
Hm. I see what you are saying about SetupTotem and .map. One of the goals I had (that I did not mention before) was for the user to be able to have a faithful imitaiton of the default totem frame with the absolute minimum of effort (in this case, "just create four frames"). But letting the user add the .Icon and .Cooldown elements and deal with positioning themselves isn't asking much.
I'm going to hold off on changing anything until you actually look at the code, but based on your feedback I am leaning towards the following: - do away with the mapping, the user can look at the SHAMAN_TOTEM_PRIORITIES/STANDARD_TOTEM_PRIORITIES tables themselves (or just ignore them) to set up the frames the way they want. - remove the code that creates .Icon and .Cooldown, letting the user create them. In the update function I will assume that if those are present, they will function as expected. - remove PostSetupTotem because it doesn't make sense to have it if I'm not creating the .Icon and .Cooldown anymore. - remove .disableCooldown because it will be redundant. I didn't even use this in the best way before anyway. - maybe rename Update to UpdateAll and UpdateTotem to Update so that (Pre|Post)UpdateTotem can become just (Pre|Post)Update - leave the default TotemFrame alone. I never had an issue with it before when I left it alone, I was thinking 'better safe then sorry' when I decided to disable it this time. Other notes: Defining things on the frame itself like totem.name and totem.duration before calling :Show() and letting the user just register OnShow would probably work, but I like how a PostUpdate would stay consistent with other oUF elements. And yeah, I started to go feature-crazy on the 'boring totembar' plugin and realized I needed to pare it back to the basics so I was not coding/supporting things that nobody (myself included!) cared about. |
|
04-06-11, 01:45 PM | #4 |
Tecu, I love your addon, just because it is so simple and "boring".
I'm really a fan of features, but please let the addon be as simple as possible! It is just great by what it is doing now, don't overcomplicate things. When you're able to add more features, please let the API stay small. I'm not really aware of what droods and DKs do about totems (shrooms, I guess for druid, but no idea for DKs), and it would possibly be nice to incorporate this into an layout, but speaking as shaman: I don't care! One thing I really liked of boring_totembar was the form of oUFs-API. Keep that in mind, please! I guess you haven't lost this focus, regarding your post here, but don't put too much into it. Perhaps it would be better to release a further plugin for other features.
__________________
|
|
04-06-11, 03:15 PM | #5 |
This is a quick suggestion of how it could become:
Lua Code:
diff: http://paste.pocoo.org/raw/366821/ It's mainly a merge of the changes discussed above, with a round of sed on top of it. Some minor notes: - I just set OnClick, OnEnter and OnLeave because: They won't work correctly if EnableMouse isn't set and such. We could re-add a post-hook here, as people might want to replace them with their own. - I don't check for unit == 'player' anymore. I expect people to be sane enough about it themselves. - Entirely drycoded changes \/.
__________________
「貴方は1人じゃないよ」 |
|
04-06-11, 04:33 PM | #6 |
@haste:
You drycoded changes are working fine in game :D PostUpdate: This is definitely needed because PLAYER_TOTEM_UPDATE only fires once when you refresh a totem. If the user only watches for OnShow, they will not know if something changes in this case. Mouse events: A post hook or .disableX option would be needed here because if any of those OnX scripts are present on the button, it will still eat mouse clicks, even if Button:Disable() and Button:EnableMouse(false) have been called beforehand. (The scripts will still not 'work', though.) In this case maybe just check for Totems.enableMouse? That way at least the user asks you to set up default mouse behavior, and doesn't have to disable anything you did if they want to do their own thing. Button: Silly question, but a frame has to be a 'Button' to respond to mouse clicks right? Question about single/double quoting strings: I thought that single/double quotes work the same way in lua, is there a reason you use one over the other? I've been tending toward the single quotes just because I am lazy. Anyway here's either a patch for your update that includes .enableMouse and the fully patched file. Dunno which of these you'd be interested in (I've used diff/patch like twice in my life before). diff: http://paste.pocoo.org/raw/368125/ patched: http://paste.pocoo.org/raw/368126/ @Mischback: I wasn't trying to add or remove things from the boring totembar plugin, but rather create something new and simple that would be very similar to what haste has been doing with other oUF elements. I have not decided anything about the future of oUF_boring_totembar; I'll have to wait and see what becomes of this first! Last edited by tecu : 04-08-11 at 01:57 PM. Reason: fixed to follow haste's if() style |
|
04-10-11, 05:17 AM | #7 | |
I don't have any plans to remove it. If it sounded like that, then it was just me being a horrible typist.
Patch below uses the same solution as the aura element. So far no-one has asked for more there (but they also have a PostUpdate afterwards, which doesn't really make sense here). Yes, Frame's don't support OnClick. Patch below allows you to use a Frame instead, if you don't want any mouse interactions. Also note that using :SetScript('OnEnter', ...) and :SetScript('OnLeave', ...) will enable mouse catching if it isn't enabled. In Lua they're equal. I wrote the code no two different laptop, one which uses qwerty and one which uses colemak. The inconsistency mainly comes from that.
--- We could just check the if the frame provided supports OnClick and if it captures mouse events: Lua Code:
__________________
「貴方は1人じゃないよ」 |
||
04-11-11, 05:29 PM | #8 |
Hey. I think this is pretty solid. The only thing I have left to add is that the Blizzard TotemFrame (unlike the RuneFrame) is parented to the PlayerFrame so you can probably just leave it alone. I only just now looked this up, sorry.
If you need or want anything more from me, just post back here. Also, thanks a ton for all the feedback and comments; it has been very helpful to me. |
|
04-12-11, 06:39 AM | #9 |
I don't see why something hidden should be updating in the background. Anyway, I'll merge the code in when I get home from work.
I'd also like to thank you for your contribution . (was planning on writing some more and posting this before 9AM, now it's closing in to 3PM... wups!)
__________________
「貴方は1人じゃないよ」 |
|
WoWInterface » Featured Projects » oUF (Otravi Unit Frames) » Totems element - looking for feedback |
«
Previous Thread
|
Next Thread
»
|
Display Modes |
Linear Mode |
Switch to Hybrid Mode |
Switch to Threaded Mode |
|
|