Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New Maps with white background cause errors #261

Closed
Zuljita opened this issue Feb 26, 2019 · 19 comments
Closed

New Maps with white background cause errors #261

Zuljita opened this issue Feb 26, 2019 · 19 comments
Assignees
Labels
bug in progress Issue is actively being worked on low Low priority bug/enhancement

Comments

@Zuljita
Copy link

Zuljita commented Feb 26, 2019

In 1.4.1.8 If you attempt to create a new map and set the color to the top left option (white) in the color palette you get the following error which I can reproduce reliably on 2 windows 10machines in very different environments:

java.lang.IllegalArgumentException: Invalid type of paint: java.awt.TexturePaint
	at net.rptools.maptool.model.drawing.DrawablePaint.convertPaint(DrawablePaint.java:38)
	at net.rptools.maptool.client.ui.MapPropertiesDialog$3.actionPerformed(MapPropertiesDialog.java:304)
	at javax.swing.AbstractButton.fireActionPerformed(AbstractButton.java:2022)
@Phergus
Copy link
Contributor

Phergus commented Feb 26, 2019

Steps to reproduce:

  1. Start Maptool. From Map menu select New Map
  2. On Map Properties Panel click on Background button
  3. On Swatches tab of Choose Background dialog, click on the white (top-left) cell in color palette. Note that background display area does not change color.
  4. Click on Okay, Exception is thrown.

Selecting any other color first, works as expected and allows subsequent selection of white cell.

@Phergus
Copy link
Contributor

Phergus commented Feb 26, 2019

Bug confirmed in 1.4.0.5, 1.4.1.8 and dev release 1.5.0.1. Java 10.0.1
Also in installer version of 1.5.0.0 with bundled Java (also 10.0.1).

@Azhrei
Copy link
Member

Azhrei commented Feb 26, 2019

Great, guys. Thanks for filing the issue!

@Phergus
Copy link
Contributor

Phergus commented Feb 26, 2019

I think this bug was been around forever and it may be that the bug is in JIDE libs but was never fixed. Long, long ago I trained myself to always click on one of the other color cells first and had forgotten about it.

@JamzTheMan
Copy link
Member

Thanks, and ya, that sounds familiar, IIRC was JRE specific. Maybe it came back?

Can you tell us, are these using "installs" or JAR versions and if later, what version of JRE used for testing? Being it's "white" it's probably not handling the 0 index of the component or color or such.

@Phergus
Copy link
Contributor

Phergus commented Feb 26, 2019

For myself, testing was with jar versions. I just tested the installed 1.5.0.0 version and it also does it. Previous post updated.

@Zuljita
Copy link
Author

Zuljita commented Feb 26, 2019 via email

@JamzTheMan
Copy link
Member

Cool, thanks for the info. Should be easy to replicate and hopefully easy to fix.
I may have to start adding some priority and sizing tags around issues...

If we can get it in for this release I'll try but i don't want to hold the release for lower priority bugs. Hopefully we can agree this is low priority as it's been around for a while now. :)

@JamzTheMan JamzTheMan added bug low Low priority bug/enhancement 1 labels Feb 26, 2019
@Phergus
Copy link
Contributor

Phergus commented Feb 26, 2019

Yeah. I think it definitely falls into the low category.

@Azhrei
Copy link
Member

Azhrei commented Feb 26, 2019

It's an interesting problem. The white cell is somehow a TexturePaint object and the code expects Color and AssetPaint (which is a subclass of TexturePaint). I thought it might be enough to broaden the check from AssetPaint to TexturePaint, but it's not that simple as an Asset is needed to create the Drawable object.

I'm leaving this for @Jaggeroth (he likely knows more about the Drawables than anyone else), but it seems like the simplest solution is to ensure that the cell in question is an Asset or Color object...

@Phergus
Copy link
Contributor

Phergus commented Feb 26, 2019

Weird that only that cell seems to be the only one with an issue.

@JamzTheMan
Copy link
Member

I wonder if this goes back to 1.4.0.0? Between 1.4.0.0 -> 1.4.0.5 is where @Jaggeroth did a lot of the drawables work. (not blaming, it's just easier to compare across commits/releases to find bugs sometimes)

@Phergus
Copy link
Contributor

Phergus commented Feb 26, 2019

Same error in MT 1.3.b91.

Bug was introduced some time after 1.2 as the option to pick background, map images and FoW came in with 1.3.

BTW, 1.2.b32 still runs under Java 10.

@JamzTheMan
Copy link
Member

FYI: As a workaround, if you pick another color and then immediately pick white, it works ok...

@sentry-io
Copy link

sentry-io bot commented Mar 18, 2019

Sentry issue: MAPTOOL-1X

@RPTools RPTools deleted a comment from sentry-io bot Mar 18, 2019
@Phergus Phergus self-assigned this May 17, 2019
@Phergus Phergus added the in progress Issue is actively being worked on label May 17, 2019
@Phergus
Copy link
Contributor

Phergus commented May 17, 2019

Thought about turning the returned TexturePaint into an asset but that just left the background the default grass texture instead of the White matching the color of the swatch clicked on. New code just forces it to be white.

@Phergus
Copy link
Contributor

Phergus commented May 30, 2019

Fixed with PR #478

@Phergus Phergus closed this as completed May 30, 2019
@Phergus Phergus reopened this Jul 23, 2019
@Phergus
Copy link
Contributor

Phergus commented Jul 23, 2019

Grrr. Replaced one bug with another. Fix in process.

@Phergus
Copy link
Contributor

Phergus commented Jul 28, 2019

Fixed in 1.5.3. Closing.

@Phergus Phergus closed this as completed Jul 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug in progress Issue is actively being worked on low Low priority bug/enhancement
Projects
None yet
Development

No branches or pull requests

4 participants