HeroDesigner.getActiveHero() -- Why does it work like this?

[Kintar]

Hey, everyone, and Simon in particular. ;)

I'm working on a (very simple) HeroDesigner plugin, and running into this behavior that doesn't seem quite right. My plugin creates an object with a com.hero.Hero class variable. When the plugin UI is activated for the first time, the Hero class variable is set to the value returned by HeroDesigner.getActiveHero(). Now, what I was EXPECTING to happen is that if the character which is active in HeroDesigner changes, my plugin object would still be holding a reference to the old Hero object. This isn't so. If the HeroDesigner object brings another Hero up as the active hero, then the values in the object held by my plugin change.

Why on Earth does it do that? :confused:

Now, I'm obviously not asking for a change to the way this works, that would just be daft. I'm only one plugin developer. :) I'm just curious why you made the design decision to change the values in the object, rather than loading in a reference to a different Hero object. I'd also appreciate any ideas you could give me to help my plugin hold a reference to the Hero that was active when the plugin was activated, since there doesn't seem to be a copy constructor on com.hero.Hero, and the clone method is protected.

Thanks in advance for your response.

-- Kintar

[Simon]

Sounds like there's something up in your code. Changing the active character in HD will change the object reference returned by HeroDesigner.getActiveHero().

The object reference that is returned by HeroDesigner.getActiveHero() is a static object in the HeroDesigner class (there can only be one in a given JVM) so you may want to look into cloning the reference that you get....

[Kintar]

Sounds like there's something up in your code. Changing the active character in HD will change the object reference returned by HeroDesigner.getActiveHero().


Well, let me show you what I've got. This is the actionPerformed code on the menu item that loads up my plugin.



Hero h = HeroDesigner.getActiveHero();
if (h == null)
{
// Clipped for brevity. Shows a popup saying you
// must load a hero before using this plugin.
}
else
{
ConditionMonitorUI ui = new ConditionMonitorUI(h);
}


Now, in the public constructor for the ConditionMonitorUI object...


public ConditionMonitorUI(Hero hero)
{
// Must be created with a hero
if (hero == null)
throw new NullPointerException();

currentHero = hero; // This is a class variable
initComponents(); // Set up the UI
setDefaultCloseOperation(JFrame.DISPOSE_ON_CLOSE);
setResizable(false);
show();
}


And the routine that gets called to repopulate the fields with the default values from the hero is...


private void getBaseValues()
{
bod = currentHero.getCharacteristic(Constants.BODY);
stun = currentHero.getCharacteristic(Constants.STUN);
end = currentHero.getCharacteristic(Constants.END);
rec = currentHero.getCharacteristic(Constants.REC);
curBod = bod.getValue(true, Constants.BODY);
maxBod = curBod;
curStun = stun.getValue(true, Constants.STUN);
maxStun = curStun;
curEnd = end.getValue(true, Constants.END);
maxEnd = curEnd;
recRate = rec.getValue(true, Constants.REC);

}


Each instance of the ConditionMonitorUI has its own reference to the hero that was currently active at the time the instance was created.

Here's the simplest case of the issue:
1. Open a character in Hero Designer.
2. Start the Condition Monitor plugin.
3. Open another character in Hero Designer. (Two loaded now, newest load is active.)
4. Retrun to the Condition Monitor plugin's window, and click the button that calls getBaseValues().
5. The values of the hero opened in step 3 are loaded into the Condition Monitor.

If the active hero variable on the HeroDesigner object gets replaced with a new instance each time the character is swapped, then there must be a bug in my code. Damned if I can find it, though. :) It's really acting as though HeroDesigner.activeHero is having its values changed, rather than getting a new object reference.

I know you've got better things to do than look at people's plugin code, but if you want a copy of the source, I'll be happy to send it to you, just so we can find out if I'm insane. ;)

-- Kintar

[Simon]

1. Plugins are not loaded by the user. They are loaded by HD automatically as the app starts, so you may want to rethink the design in general to prevent people from getting error messages that they have little to no control over.

2. The active hero in the HeroDesigner class is a static variable. Your code is likely grabbing that static reference and changing as the static variable is changed. Look into either cloning the instance returned or working on a different scheme entirely (per 1, above).

[Kintar]

1. Plugins are not loaded by the user. They are loaded by HD automatically as the app starts, so you may want to rethink the design in general to prevent people from getting error messages that they have little to no control over.


The plugin does indeed get loaded by HeroDesigner, I was just skipping over a few steps for the sake of brevity. The main plugin class (the one referenced by Main-Class in the manifest of the jar file) does the gruntwork of adding a new menu to the main menu bar. The menu items in there call up the various UI elements I've written, such as the ConditionMonitor.




2. The active hero in the HeroDesigner class is a static variable. Your code is likely grabbing that static reference and changing as the static variable is changed. Look into either cloning the instance returned or working on a different scheme entirely (per 1, above).


So what you're saying is that the static variable in the HeroDesigner class holds a reference to a single object, and the values in that object are changed, rather than the static variable being changed to point to a different object with the new values, correct?

If that's the case, then my original question comes back into play: Why did you design it that way? I'm not saying it's right or wrong, I've been party to many debates about object references (pointers) versus actual data in my C++ days. I'm just curious as to the reasoning behind the design decision.

Anyway, since the reference returned by getActiveHero() is always a reference to the same object, you may want to consider making a publicly accessible clone() method on the Hero object. As it stands, Hero.clone() is protected, so plugin designers have to create their objects in the com.hero package to allow the plugin to create copies of a referenced Hero. Not a big deal, just an unnecessary step. :)

Thanks for the replies, Simon. And keep up the great work! This is the ONLY computer character design tool I've ever used that works the way it's supposed to.

-- Kintar

[Simon]

That's not what I'm saying, actually....the code may help:
<pre>
public static Hero activeHero;

<b>LOTS</b> of other stuff...

public void setActiveHero(Hero hero) {
{snip}
activeHero = hero;
{snip}
}

public static Hero getActiveHero() {
return activeHero;
}
</pre>

[Kintar]

Okay. This is the way I was expecting it to be set up. Progress! =)

For sanity's sake I was re-testing the plugin, and ended up using a different pair of character files. It appears that my initial thought ( that the values were being set to the current character's values ) was the product of a weird coincidence with the original characters I was using. In using these different character files, it's apparent that not everything is coming across wrong. Only the figured characteristics are behaving oddly.

Rather than continue to babble about this, find attached a zip file containing the plugin and the character files I'm using to test. The subdirectory under plugins is required. (I'm expecting to put more plugins out over time, so I needed a way for the plugin to load plugins. *sigh* I really need a cleaner way to do this. <grins>) I'll just feel better when someone else can look at it and go "Yep, that's weird."

Source code on request; I'd rather not post that to the forum.

-- Kintar

[David Akers]

My guess is that your code should actually look like this:

Hero h = new Hero(HeroDesigner.getActiveHero());

I think that with out the "new Hero" command you are getting a reference rather than the actual object. I've run into that in other things, myself.

-David

[Simon]

My guess is that your code should actually look like this:

Hero h = new Hero(HeroDesigner.getActiveHero());

I think that with out the "new Hero" command you are getting a reference rather than the actual object. I've run into that in other things, myself.

-David Not quite. The reference is changing within the HeroDesigner class (shown in the code above).

What he's running into is a problem that he really had no way of knowing about: many of the calculations that need to be done on a character can only be done when that character is "active" -- they rely on looking up values in other areas of the character. Figured Characteristics (and Characteristics in general) would be one of these areas.

You'll need to set the hero to active in order to use it. The program is not designed to perform calculations on inactive characters.

[Kintar]

You'll need to set the hero to active in order to use it. The program is not designed to perform calculations on inactive characters.


I had the rising fear you were going to say something like that. :weep:

Okay, it'll just take a little fiddling. I'll load all the stats I need into the ConditionMonitorUI class, and change the GUI around to allow a reset to those values, or a reload from active character.

Thanks for the info.

-- Kintar

[Kintar]

Okay, almost done here. :D

I've changed the plugin around to do the following:



private void getBaseValues()
{
Hero temp = HeroDesigner.getActiveHero();
HeroDesigner.getInstance().setActiveHero(currentHe ro);

bod = currentHero.getCharacteristic(Constants.BODY);
stun = currentHero.getCharacteristic(Constants.STUN);
end = currentHero.getCharacteristic(Constants.END);
rec = currentHero.getCharacteristic(Constants.REC);
maxBod = bod.getValue(false, Constants.BODY);
maxStun = stun.getValue(true, Constants.STUN);
maxEnd = end.getValue(true, Constants.END);
recRate = rec.getValue(true, Constants.REC);
curBod = maxBod;
curStun = maxStun;
curEnd = maxEnd;

HeroDesigner.getInstance().setActiveHero(temp);
}


Everything is now working ( :thumbup: ), except the recovery stat. Recovery is still coming back as the recovery for the hero that was active before the call to setActiveHero(). :think:

Any ideas, since I'm now pulling the stats for the actual active hero?

-- Kintar

[Simon]

Please verify that...there's nothing special about REC in the system (it's just another Characteristic as far as HD is concerned). There is no reason for it to be acting any differently from the others.

The only thing that I can think of other than a misinterpretation on your end is a problem with the caching of the value, but we should have run into that long ago if it were happening on the REC Characteristic.......

Anyway, check it out and verify that you're seeing what you think you're seeing....if so, then I'll dig through the caching code to see if I can identify any problem areas....

[Kintar]

Yeah, after my last fubar in the vision department, I double-checked this one while waiting for a reply. It's hard to describe exactly what it's doing, as the recovery value is definitely based on the previously loaded character, but isn't the final recovery value. Here's the latest plugin (rename to .jar, of course, and drop it in the infinities subdir), and the two characters I was using to test.

-- Kintar