An ItemMenu interface engine

I've decided to embrace vJass quite heavily and tried to create an OO tool to simplify the annoying process of creating item Menu's.

The big problem with ItemMenu's is handling state and the more systems you have the more case works arounds you need.
The Item menu's themselves weren't very tricky. It was having to deal with interoperability that sucked. I've tried to simplify that by making itemMenu's an interface. Each itemMenu has a nice onOpen() action, and a probably more important onInterupt() action. But I didn't stop there I added event responses for some of the most common item menu related actions. Please give me comments and suggestions

Edit revision 1:
Learning a skill forwhatever reason triggered a no-target instant order with orderid=0 and could trigger the helloWorldState. I've fixed this by only allowing orders of positive values to be used as m_openAbilityOrder. Entering 0 should mean it is ignored.

Er, it doesn't look too user friendly. One of the benefits of interfaces is that you can hide everything ugly from the user.

1.) The interface should contain only variables the user cares about. Hide everything else in a struct that contains the interface struct as a member.
2.) Event response variables should be passed as parameters to the methods
3.) Things that you "must define" when you create an item menu should be parameters to the create method

Can you explain what all of this "state" stuff is? It seems like something that should not be necessary or at least completely hidden from the user.

Um, does it usually use the item in slot #7 (top left) when u learn a skill? I find that quite strange. It instantly used the item that opens the menu when i learned any skill on the paladin. Yet inside the menu when i learned a skill it didn't use any item cause no text was printed.

1) Thats kind of true. The interface for the menu calls could just be the functions. But I liked the idea of having all those variables available at any given time.
2) Err, that just seems messy. Unfortuantly there could be cases where event responses will not be populated (i.e. see .onOpen() )
3) Thats entirely possible. But since the create method is not part of the interface it doesn't really matter.

The state is a neccessary evil. Essientially its how to convey what itemMenu is open.
State -1 is the escape state, so this deactivates all itemMenu triggers.
State 0 is the nuetral state. This is the state when no item Menu is open.

From then on you define states. My hello world state is 1. All these itemMenus are stored on the hero object in his array m_states.

It kind of sucks I realize but there are cases where you don't want to go back to state 0 when you leave a trigger. A simple sort of implementation is just to create a global variable so that you can edit the state (that way if you wanted to add the example helloWorldMenu but you already implemented something as a state 1 object you could just change the variable of the helloWorldState).

Obviously in this case it isn't really neccessary to know an explicit state. When there are multiple systems it becomes a dire need.
Edit:
Fixed the bug!

the important thing to note is how very little code is actually written in Hello World Menu. If you even barely understand Jass you'll see that the Hello World Menu's code has no useful code at all, yet there are many things handled (like what happens when you swap the items, or when you drop the items or hit cancel).

The variables are still available to use, I don't know what you're talking about.

How exactly are struct members which "may or may not be populated" better than parameters which "may or may not" be null or 0?

In the examples in your map you had plenty of ugly code within the create functions, that should be hidden from the user somehow...

Er, if you want to convey which item menu is open, then stick the pointer to the item menu into a member called "currentMenu" or something. I'm still not seeing the need for this state variable.

State It allows interoperability and has to be known at compile time as sometimes Menu's want to quit into a different state other than nuetral state. State is essientially current menu. hero.m_state is the current menu open. hero.m_states[hero.m_state].m_state is a silly line. but it also will return the current menu.
m_states hold all the item menu's for this hero. An itemMenu also has to know if its triggers should run. The way it does that is by looking at its own state variables and comparing it against the state variable of the hero it wraps.
Every itemMenu has a trigger for having an order targetting a widget. That would mean a lot of .onItemUse() events will fire. But since I catch it early if(h.m_state==handler.m_state) Most of the triggers quit before any expensive code runs. (Only trivial code runs like one gamecache restore call and some local variable assignment)

State may be a terrible name for this. But this was the old naming convention I used.



The only really ugly code is populating the the internal item vector and assigning the handler a hero and visa versa. I guess you could throw all the configurations .m_ as parameters to .create() but really I feel those are pretty static and aren't going to change from hero unit to hero unit. Those could be defined as global variables so the user doesn't have to actually manually edit the create method.
The create method takes the only parameter that I think matters, the hero object to assign it.

Then a menu object should have a variable for which menu it quits to. Your way of doing this is way counterintuitive. Each menu object already has a unique integer all to itself (typecast to integer) yet you're assigning arbitrary integers to represent menus as well.

Yes, that's the kind of silly line that a user should not have to look at.

And how is this different from using the "currentMenu" variable?

That's not the point, the point is that there's no reason to have that code exposed to the user.

Anyway if you're serious about embracing vJASS it may be necessary to take a step back and consider the huge number of design options there really are for any given situation; I'm sure that there's a much more elegant way of doing all of this.

I should say this is the work of about 4 hours and some thinking so its not the greatest peice i've written. I just liked how quick I was able to work it out.
I'm not sure its such a good idea to make this so headless so that the hero doesn't need an array of all his menus. There is also the issue of EscapeState which isn't a real menu and doesn't have an associated object.

The above line is completely retarded And you don't need it ever.I was trying to illustrate the circular reference chain. If I get rid of m_state and replace it with a variable like m_currentMenu, but stilll keep m_states (possibly replace it with m_itemMenues) we still can produce absoultely wretched lines like
.m_itemMenues[.m_currentMenu].m_hero.m_itemMenues[.m_currentMenu].m_hero.m_currentMenu
Hell I could even get rid of the itemMenues var and get this horrible stupidity.
.m_currentMenu.m_hero.m_currentMenu.m_hero.m_currentMenu //repeat forever and ever.
But thats the innate flaw in any system with circular reference.Circular reference isn't a bad thing in my opinion.



I'm going to be doing some rehauls over this weekend when I get time. I'm going to remove the hero.m_iv item array, and I'll use nuetralState's iv since its not really using it and it has to be on your character anyway. I'll also add a non-trivial system probably attributer.

I will probably move the event responses out into parameters. It makes it easier on knowing what parameters you could be using.
I'll also add a customtrigger method to the interface that'll be called during initItemMenus.

edit:
anyState would be annoying too. I guess >1 works but eww is that ugly and is an implementation requiring nonsense.

edit2:
What would you suggest as a work around from populating an item vector with items? This is like editting a bunch of spellbook ability in the object editor it annoying. But you just copy the object from a different map. Here you just copy the code into your map. Once you have the item menu code you can just copy in any item menu and give it a unique state. Not every itemMenu will also have an explicit item vector. A bag would not have one and each bag item would have its own vector

itemMenu.addItem(item)
hero.addMenu(menu)
something like that, it should all be encapsulated within functions or methods with very clear usages.

Make it 0 then.

itemMenu is an interface so I can't make a method add the item. The whole item hiding shebang could be encapsulated in item vectors methodology but there are times when hiding isn't neccessary and would only confuse the situation and annoy the end user.

You'll have to do itemMenu.m_iv.add(item). I guess I could rename add to addItem but thats just being pandactic. Also remember that its not likely you'll be adding to a menu. An attributer isn't going to get new items. and when thats the case you could just add a wrapped function like addItem that all it does is