Tracker ids 3463, 3465 and 3472

Thus spake bobd:

I have committed fixes for the following to bobd-bugfixes3.1

Bug 3463 - Infinite recursion if $PieceName$ specified as Text Label
SVN revision 7658
I think I am the only one who has hit this one.

Bug 3465 - Empty DynamicProperty causes IllegalStateException on loading
SVN revision 7659
This was traced to a propblem with SequenceEncoder that I think is the
culprit for many of the “IllegalStateException No state for Decoder”,
and the older “NoSuchElementException in
SequenceEncoder.Decoder.nextToken”, bugs.

Bug 3472 - Initial value of DynamicProperty is not getting evaluated
SVN revision 7660
This was something I wanted, no one else has asked for it.

I’ll try to review these soon. I’m finding that I’m being overtaken
by events right now…

Should I mark these as TRIAGED? (I only have the option of TRIAGED,
ASSIGNED OR RESOLVED)

TRIAGED indicates that someone has looked at the bug, verified that it
is a bug, and also not a dupe of an existing bug.

ASSIGNED indicates that someone has comitted to working on the bug.
(However, take this with a grain of salt—there are many bugs assigned
to me which I intend to work on, but I’m not working on presently.)
If you’re working on a bug, you should also assign it to yourself. Right
now, everything is assigned to me by default.

RESOLVED means that the bug is fixed and code committed to the repo.

And re: bug 3465 - Do you want me to mark those I think are duplicates
as such?

Yes, please do.


J.

Thus spake bobd:

I have committed fixes for the following to bobd-bugfixes3.1

I started looking at these, but ran into a problem: You didn’t say
which revisions on your branch contain these fixes, and you didn’t
include any log messages with your revisions so that I could tell
what the changes were for.

Which revision goes with which bug?

J.

Sorry about no commit message, the eclipse plugin didn’t prompt me for a message and I didn’t think about it until after I had committed all of the changes. I will have to research how to add commit messages before I do anymore work.

Bug 3463 - Infinite recursion if $PieceName$ specified as Text Label
SVN revision 7658

Bug 3465 - Empty DynamicProperty causes IllegalStateException on loading
SVN revision 7659

Bug 3472 - Initial value of DynamicProperty is not getting evaluated
SVN revision 7660

Hopefully that is the information you need.

Bob

Thus spake bobd:

Sorry about no commit message, the eclipse plugin didn’t prompt me for a
message and I didn’t think about it until after I had committed all of
the changes. I will have to research how to add commit messages before
I do anymore work.

Bug 3463 - Infinite recursion if $PieceName$ specified as Text Label
SVN revision 7658

Bug 3465 - Empty DynamicProperty causes IllegalStateException on loading
SVN revision 7659

There’s a problem with this commit, namely that you’ve entirely replaced
SequenceEncoder.java instead of commiting just the changed lines. I’m
not sure how you did that, as it’s not what I would expect to happen by
default.


J.

Thus spake Joel Uckelman:

Thus spake bobd:

Sorry about no commit message, the eclipse plugin didn’t prompt me for a
message and I didn’t think about it until after I had committed all of
the changes. I will have to research how to add commit messages before
I do anymore work.

Bug 3463 - Infinite recursion if $PieceName$ specified as Text Label
SVN revision 7658

Bug 3465 - Empty DynamicProperty causes IllegalStateException on loading
SVN revision 7659

There’s a problem with this commit, namely that you’ve entirely replaced
SequenceEncoder.java instead of commiting just the changed lines. I’m
not sure how you did that, as it’s not what I would expect to happen by
default.

I see what’s wrong: You’re saving files with DOS newlines instead of
UNIX ones. All of our source files use UNIX newlines; if you’re using
DOS newlines, it screws up diffs: What I was seeing when I merged this
change was that every single line in the file was different—due to
to each ‘\n’ having been converted to ‘\r\n’.

There’s a setting in Eclipse which you can change to avoid this problem:

sics.se/node/2108


J.

Thus spake bobd:

Bug 3465 - Empty DynamicProperty causes IllegalStateException on loading
SVN revision 7659

This looks ok to me. Have you checked around to see if you can find any
modules which were relying on the old behavior?


J.

Thus spake bobd:

Bug 3465 - Empty DynamicProperty causes IllegalStateException on loading
SVN revision 7659

Merged to 3.1@7664.


J.

Thus spake Joel Uckelman:

Thus spake bobd:

Bug 3465 - Empty DynamicProperty causes IllegalStateException on loading
SVN revision 7659

This looks ok to me. Have you checked around to see if you can find any
modules which were relying on the old behavior?

BTW, this old thread is related to this bug:


J.

Sorry about the newline thing, I will look at my other changes and make sure all are just LFs.

I missed one issue in Vassal, Embelishment.oldGetType(). Nothing in Vassal uses it but I suppose some custom module might so I should make a change to that as well. Sorry about that.

I haven’t checked non-Vassal code for use of SequenceEncoder, which modules should I check?

Thus spake bobd:

Sorry about the newline thing, I will look at my other changes and make
sure all are just LFs.

I missed one issue in Vassal, Embelishment.oldGetType(). Nothing in
Vassal uses it but I suppose some custom module might so I should make a
change to that as well. Sorry about that.

This is in reference to which of the three bugs?

I checked the log for Embellishment. Embellishment.Ed.oldGetType()
I deprecated in 4062, back in 2008. It might well have been an
oversight that I didn’t also deprecate Embellishment.oldGetType() at the
same time.

Generally, we do want to keep deprecated code working until we remove
it, to give anything which depends on it time to stop relying on it.

I haven’t checked non-Vassal code for use of SequenceEncoder, which
modules should I check?

Practically any module with custom Buildables would use it.


J.

Have fixed the newlines, the tests had Windows newlines so I have changed those in rev 7666.

Embellishment.oldGetType() is fixed in rev 7667

I don’t wish to appear stupid (ha!) but how do I know which modules have custom buildables? Do you download all the modules on the Vassal site to keep an eye on what custom code there is in use?

Oh, I didn’t answer your question about oldGetType() - thats a bug 3465 (SequenceEncoder) issue as it called
new SequenceEncoder(null, ‘;’)

Thus spake bobd:

Have fixed the newlines, the tests had Windows newlines so I have
changed those in rev 7666.

Embellishment.oldGetType() is fixed in rev 7667

I don’t wish to appear stupid (ha!) but how do I know which modules have
custom buildables?

It’d not a dumb question. The only way to tell is to check each module.
I’m not suggesting you do that. An easier way would be to ask some of
the more knowledgable module designers for suggestions of what to check.
I’m affraid I can’t offer any suggestions myself here.

Do you download all the modules on the Vassal site
to keep an eye on what custom code there is in use?

No. Right now we have no idea how much custom code is out there.


J.

Thus spake bobd:

I have committed fixes for the following to bobd-bugfixes3.1

Bug 3463 - Infinite recursion if $PieceName$ specified as Text Label
SVN revision 7658
I think I am the only one who has hit this one.

Merged to 3.1@7668.


J.

Thus spake bobd:

Oh, I didn’t answer your question about oldGetType() - thats a bug 3465
(SequenceEncoder) issue as it called
new SequenceEncoder(null, ‘;’)

Merged to 3.1@7670.


J.

Thus spake bobd:

Bug 3472 - Initial value of DynamicProperty is not getting evaluated
SVN revision 7660
This was something I wanted, no one else has asked for it.

Hey, Tim? Do you see that this one will cause any problems with
existing modules?


J.

Thus spake bobd:

Sorry about no commit message, the eclipse plugin didn’t prompt me for a
message and I didn’t think about it until after I had committed all of
the changes. I will have to research how to add commit messages before
I do anymore work.

Bug 3463 - Infinite recursion if $PieceName$ specified as Text Label
SVN revision 7658

Bug 3465 - Empty DynamicProperty causes IllegalStateException on loading
SVN revision 7659

Bug 3472 - Initial value of DynamicProperty is not getting evaluated
SVN revision 7660

Hopefully that is the information you need.

Bob

What does 7661 belong with?


J.

Thus spake bobd:

I have committed fixes for the following to bobd-bugfixes3.1

Bug 3472 - Initial value of DynamicProperty is not getting evaluated
SVN revision 7660
This was something I wanted, no one else has asked for it.

I think this is a bug, actually.

Merged to 3.1@7672.


J.

Bug 3479 - ABR: ClassCastException: java.lang.Boolean cannot be cast to java.lang.String

7661 was most of the work but I missed something which meant I had to do another commit (7662)

No I don’t think so - but I know what to test when it comes out :slight_smile: . It’s an enhancement Bob wanted like he says, not a bug. Custom mods should rely/worked with the basic behavoior of DP and not expect initial value to evaluate.