On Tue, 14 Dec 2021 20:24:35 +0100 (CET) Stefan Fellner <stefan.fell...@tutanota.com> wrote: > I am not sure what you mean with the graphics files - if you are referring to > the icons (cigars_import.png, ...), I have added them on purpose - they are > the original icons plus an arrow green/red for the direction.
OK, I misunderstood what they were there for. However adding specific
modified goods icons is not a good idea as we have multiple sets of
graphics --- mods can install their own versions. What I think needs to
be done here is to use the currently specified goods icons and either
place arrows next to them or even modify the images on the fly in game ---
see ImageLibrary.getMissionChip for an example where create an image with
a cross in it, and ClassicMapControls for existing arrow resources to
reuse.
> I redesigned the trade route handling to be able to define for each stop what
> to load and what to unload.
Is that not already clear? The existing stop shows what leaves the
colony. Anything not present gets unloaded. Having to specify things
twice sounds more error prone.
> For loading the export limits are still valid.
If you want to change the logic around goods loading/unloading in
InGameController that is a separate issue. As far as I can tell, that
part is working fine, albeit the code is complex and not pretty.
> I noticed that you have something called "enhanced trade routes" - can you
> explain what this is?
If the enhanced trade routes option is enabled you can specify an
*import* level for a goods type, such that the colony/trade-route will try
to retain that many goods of that type, but not more. This was motivated
by the situation in the late game where your colonies have nothing better
to do than produce artillery, but have insufficient tools production, in
which case players like to send wagons out from the tool-producing
colonies to deliver tools to nearby artillery builders, but only unload
enough tools to so that the colony can complete an artillery unit (40),
retaining any excess tools to take to the next colony on the trade route.
This is still a bit experimental, and adds significant complexity, but was
working in my last large game a while back. Do you see now why I am wary
of changes to the goods load/unload logic? Its very easy to break, trust
me on that, I have, repeatedly.
> > For example, the modality hackery is potentially useful...
>
> Mm, I can extract that part too; however as this fixes part of the issues
> with the TRIP
Agreed. It may fix BR#3209, I am a poor judge of that as I have never
been able to replicate it. However, the general layout and dragging of
goods and stops around on the TRIP is also broken and thus a separate
issue. What I wonder about the modality problem is why it does not also
show up in other places we do string input, like ChatPanel for example.
Maybe it does.
Looking at the general problem... we already have a nasty hack in
FreeColAction:
/**
* Checks if this action should be enabled.
*
* @return True if the {@link ClientOptionsDialog}
* is not visible.
*/
protected boolean shouldBeEnabled() {
return !getGUI().isClientOptionsDialogShowing();
}
Ugh, a special case for one dialog. Looking at this makes me think what
would be better is if the ActionManager could maintain a list of
active modal panels/dialogs, including the ClientOptionsDialog[1] and
TradeRouteInputPanel. The panels themselves would register with the
ActionManager when they are displayed, and have a closing callback to
deregister themselves. That gives us a general mechanism, and the special
case above becomes !getActionManager().modalsRegistered() of the like.
We may even be able to get away with just a reference count
increment/decrement.
>>[Unused loggers]
> I tend to disagree - if anyone needs a logger (again) it's easy to add it;
> why keep unused stuff around?
It is easy to add/remove, but that is still non-zero effort. Call it
enlightened laziness. However no one is going to argue over this if you
want to do really low priority work.
> Plus my eclipse shows all unused stuff (e.g. private members) as warnings,
> and i don't like the source file be yellow blinking :P
This is indeed sometimes useful to show where we have dead code. I know
we have a few get/set pairs where the setter is not currently used, but if
you see something weighty that is no longer called that is worth fixing.
> > I am curious though, why do you use lambda's in loggger
> > messages?
> >
> Ah, the lambda stuff for loggers - my sonar complains - reason is that if the
> level is too low, the expression get's only evaluated in case it really needs
> to log something - this is really only a minor improvement (unless huge
> amount of logs). Sure, the System.err etc. should be replaced with the
> loggers (or removed).
> If you want, I could sweep the code base (simple search should locate the few
> matches, easy fix).
There are a couple of System.err uses that are correct because
the logging system is not reliably initialized at point of call, but the
most common reason is developer error. Feel free to delete on sight.
Wrt lambdas, OK I wondered if it was an optimization. FreeCol does do a
lot of logging, and indeed much of it is at low levels. I wonder how much
effect there would be by a global change to logging-with-lambdas?
> > - Apparently you do not share my appreciation for the ternary operator:-)
> Uhm ... depends. If you have one operation like in a return ("return valid ?
> unit : null;") this is great.
> But if you start to stack them, this becomes quite unreadable in my opinion.
Just think of it as an if-then-else chain that takes up less vertical
space:-). Or that the author really likes Lisp.
Cheers,
Mike Pope
pgpqsPUYRI7Mi.pgp
Description: OpenPGP digital signature
_______________________________________________ Freecol-developers mailing list Freecol-developers@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/freecol-developers
