search

yellowstonegames / SquidLib

Useful tools for roguelike, role-playing, strategy, and other grid-based games in Java. Feedback is welcome!
Other
454 stars 46 forks source link

Regression in SquidPanel #123

Closed smelc closed 8 years ago

smelc commented 8 years ago

In commit 2073750630a5efa0ef7c0e8351da085c94a36f1c, a constructor of SquidPanel was changed: instead of taking the given TextCellFactory (as the documentation says), it copies it:

https://github.com/SquidPony/SquidLib/commit/2073750630a5efa0ef7c0e8351da085c94a36f1c#diff-a393ed66cf045402276c1d64d0e22d34

I don't know if this is intended, but it breaks my game, from (terminal shows the current revision at its top):

squidlib-bb518cfd1567e015533dac59d4de04e7d9fd3e54

to:

squidlib-2073750630a5efa0ef7c0e8351da085c94a36f1c

Somehow the font's or the factory's settings broke :-( The copy of the factory does not have the intended effect :-(

tommyettinger commented 8 years ago

I think this is caused by making changes to a TextCellFactory after it has been assigned to a panel or other GUI element. I have another way to accomplish the same thing the original copy was meant to do, without causing this kind of break... I think. I'll remove the TextCellFactory.copy in SquidPanel or any other places that do that where it isn't currently expected, and push it into the DefaultResources getters so modifying like with TextCellFactory font = DefaultResources.getStretchableFont().width(1).height(1000).initBySize(); won't completely ruin anything else that just gets the default with DefaultResources.getStretchableFont();. I think it will construct and store one and just return copies when requested from DefaultResources, but SquidPanel will not copy again (which should fix this issue).

Working on this now.

tommyettinger commented 8 years ago

Latest commit may fix this, not sure. SquidPanel no longer calls copy() on TextCellFactory objects passed to it; TCFs obtained through DefaultResources are copied from one stored instance there. BitmapFonts obtained from DefaultResources never changed. There may be some cases where copying manually is needed, and this is done in EverythingDemo to avoid changing text sizes for all SquidPanels to the last size used (which was very small, and looked weird). It turns out copying should be done after initializing with a TextCellFactory's initBySize() or initByFont() methods to make certain things that expect a size from a TCF will work.

smelc commented 8 years ago

This fixed the issue, thank you.