Bug 2817070: PropertySheet saves empty hotkey as null

I’ve investigated this bug and found the cause:

sourceforge.net/tracker/index.p … tid=594231

What it amounts to is that PropertySheet lets you set its hotkey to
nothing, which results in launchKey being set to zero, and that’s written
out when you save the module as the XML entity for the null character,
which is #&0.

The error which is finally thrown happens when you load the module
containing the #&0, but it’s not wrong to throw an error due to that: #&0
isn’t an entity which should appear in our XML documents.

The solution has to be to keep PropertySheet from writing out a null
entity in the first place, but I’m unsure about how to do that. Do I need
to modify PropertySheet.myGetType()? I see this:

se.append(m_definition).append(menuName).append(launchKey).append(commit).
append(red).append(green).append(blue);

If launchKey == 0, what should we write out instead?

The correct solution is deprecate the use of editing launchKey as a char and use a proper Hotkey Configurer like every other Decorator in Vassal.

I would add an additional field to state, launchHotkey, edited and stored as as a standard Hotkey and make sure launchKey is output as space from now on. updateFieldsFromState() can do some fancy footwork to determine when launchKey has been specified, but not launchHotKey and do a conversion.

se.append(m_definition).append(menuName).append(“”).append(commit).
append(red).append(green).append(blue).append(launchHotKey);

Rgds,
B.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Thus spake “Brent Easton”:

Do you mean “space” or “empty string” here? You say space in the text,
but the code has an empty string.


J.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

*********** REPLY SEPARATOR ***********

On 13/07/2009 at 3:22 PM Joel Uckelman wrote:

I meant empty string, but either will do since we will not be using the entry in the type anymore. It’s just a space filler kept for backward compatibility.

B.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Joel,

Do you want me to look at this one? I know exactly what needs to be done.

Also, swampwallaby-3.1@5847 finally nails the NPE’s in FreeRotator.

Bug [2259361] NPE in FreeRotator - Counter moved or deleted during rotate

The last fix for this didn’t go far enough, it was still possible for a counter to be moved or deleted while the interactive rotate was in progress via Global Key Commands.

B.

On 13/07/2009 at 8:36 AM Brent Easton wrote:


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Thus spake “Brent Easton”:

Sure.

I left a note for you about this attached to the bug. I think it might
make more sense to have a way of blocking input generally, rather than
having an ad hoc solution just for FreeRotator.


J.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Sure, in a perfect world, but there are another 50 bugs to be looked at:)

Also, blocking more ‘bad things’ that might happen while an interactive rotate in progress is a never ending slippery slope. Essentially, you need to turn the Free Rotate into a modal operation, which is difficult the way it is implemented.

The fix I put in is reliable - FreeRotator checks its own state before doing any work and cleanly aborts and tidies up if something has gone wrong.

B.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Thus spake “Brent Easton”:

I don’t mean that your fix is unreliable for what exists now, just that
doing it this way is inherently brittle, since we could add things which
affect pieces being rotated that this won’t cover.

In the long run, I think that reimplementing it so that it is truly modal
is the right way to go, but that’s pretty far down my list of things to
do at present.


J.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Swampwallaby-3.1@5851 has fix for

Bug 2817070 PropertySheet saves empty hotkey as null

Swampwallaby-3.1@5852 has fix for

Bug 2817139 NPE in RangeFilter ctor

These will be messy to merge into the trunk, I’ll look after it.

B.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Thus spake “Brent Easton”:

Merged to 3.1@5855.

Thanks.


J.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)

Thus spake “Brent Easton”:

Merged to 3.1@5860.


J.


Messages mailing list
Messages@forums.vassalengine.org
forums.vassalengine.org/mailman/ … engine.org

Post generated using Mail2Forum (mail2forum.com)