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