Re: [Freecol-developers] TradeRouteInputPanel patch
On Wednesday, December 15, 2021 12:33:10 AM EST Michael T. Pope wrote: > On Tue, 14 Dec 2021 20:24:35 +0100 (CET) > Stefan Fellner wrote: > > 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. > If I can add my two cents, I'll say that as a user, this is one of the things I've found most confusing about the game. I'm never quite sure whether I'm loading or unloading at each stop. I frequently redo trade routes several times to try testing whether they are working the way I expect and usually get it wrong the first time. It's very exciting to see so much activity here again. I have a brief window of availability before the Spring semester starts in which I can perhaps contribute small fixes or test things you'd like to see explored a bit more. If there's some way I can help that's more involved, I'd be happy to find a student who would want to work on it with me for a project this summer. Thanks! Robert (atomopawn) ___ Freecol-developers mailing list Freecol-developers@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/freecol-developers
Re: [Freecol-developers] TradeRouteInputPanel patch
Hi all, I will rework the TRIP patch and split it up into smaller parts. 1.) will be the fix for the issue that the (non-modal) TRIP panel where you can enter the traderoute name, that there are no shortcut actions triggered. As Mike already suggested I will try to do it in a generalized way, so to be able to reuse the mechanism on other panels. 2.) I will fix the TRIP functionality as it is, without changing the current trade route system. 3.) If you agree I will try to make a suggestion for a new handling / traderoute system along the lines of my patch / and what also wintertime suggested. Or maybe we should first talk about the design :P We can also skip 3 (for now) because it may not be required for a release, and convert to an improvement we can discuss later. However I would see it also like wintertime - a more flexible trade route design might be wishful. Combined also with the extended trade route feature mike already explained to me. > - default (do nothing, tbd if empty stops would be skipped or route followed > to avoid enemies) > - load (with or without limits) > - unload (with or without limits) > - Other games may also need direct buy and sell (or maybe for Europe or > Native auto trading?). > Wintertime / Mike you could take a look at the TRIP with the patch i sent - in the game, if this direction is good or not. For my tests it was working. The icons I added could also be skipped alltogether, you see whats loaded/unloaded if you select the stops anyway. For the points from wintertime, load (limits defined by warehouse export), unload (without limits) was working. selling in europe should work (buying i didn't test but should also work). Auto trade with natives (or other europeans?) may be an additional feature, but would require some work (what about the haggling, interaction with europeans / multiplayer?). BR, Stefan! Dec 15, 2021, 08:18 by win...@genial.ms: > Hi, > >> Gesendet: Mittwoch, 15. Dezember 2021 um 06:33 Uhr >> Von: "Michael T. Pope" >> An: "Stefan Fellner" , "FreeCol Developers" >> >> Betreff: Re: [Freecol-developers] TradeRouteInputPanel patch >> >> On Tue, 14 Dec 2021 20:24:35 +0100 (CET) >> Stefan Fellner wrote: >> > 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. >> > I had no time to look at the patch, but from playing other games like > Patrician II, > a trade route stop needs a 3(5) way choice for each good (maybe button near > each line to toggle the 3 > or have default be nonexisting line while allowing to add a good to the > action list for that stop): > - default (do nothing, tbd if empty stops would be skipped or route followed > to avoid enemies) > - load (with or without limits) > - unload (with or without limits) > - Other games may also need direct buy and sell (or maybe for Europe or > Native auto trading?). > This should allow routes where you collect some goods but bring everything to > the hub city. > Additionally, you can allow for more than 1 stop or doubled goods in the > action list for one stop > at same place for prioritizing cargo types through unload+load, if you allow > dragging up and down > in the action list for each stop, as the space in a wagon/ship can get too > small. > > Greetings, > > wintertime > ___ Freecol-developers mailing list Freecol-developers@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/freecol-developers
Re: [Freecol-developers] TradeRouteInputPanel patch
Hi, > Gesendet: Mittwoch, 15. Dezember 2021 um 06:33 Uhr > Von: "Michael T. Pope" > An: "Stefan Fellner" , "FreeCol Developers" > > Betreff: Re: [Freecol-developers] TradeRouteInputPanel patch > > On Tue, 14 Dec 2021 20:24:35 +0100 (CET) > Stefan Fellner wrote: > > 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. > I had no time to look at the patch, but from playing other games like Patrician II, a trade route stop needs a 3(5) way choice for each good (maybe button near each line to toggle the 3 or have default be nonexisting line while allowing to add a good to the action list for that stop): - default (do nothing, tbd if empty stops would be skipped or route followed to avoid enemies) - load (with or without limits) - unload (with or without limits) - Other games may also need direct buy and sell (or maybe for Europe or Native auto trading?). This should allow routes where you collect some goods but bring everything to the hub city. Additionally, you can allow for more than 1 stop or doubled goods in the action list for one stop at same place for prioritizing cargo types through unload+load, if you allow dragging up and down in the action list for each stop, as the space in a wagon/ship can get too small. Greetings, wintertime ___ Freecol-developers mailing list Freecol-developers@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/freecol-developers
Re: [Freecol-developers] TradeRouteInputPanel patch
On Tue, 14 Dec 2021 20:24:35 +0100 (CET)
Stefan Fellner 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
Re: [Freecol-developers] TradeRouteInputPanel patch
On Sun, 12 Dec 2021 21:35:42 +0100 (CET)
Stefan Fellner wrote:
> I have been working on TradeRouteInputPanel the last week and I got it
> running now.
Er, the attached patch contains a lot of stuff unrelated to the TRIP.
I am pretty sure the changes to the graphics files are not what you wanted!
I suggest you rebuild the patch, or rather, break it up into multiple
patches, each of which does *one* thing.
For example, the modality hackery is potentially useful, but I am
concerned that isModalFreeColPanelShowing() may turn out to be brittle
(are you *sure* notifyClose covers *all* cases?). If we need to revert
this change, I would prefer it be able to be done easily without also
reverting the fixes to TRIP. Similarly, while
InGameController.unloadUnitAtStop is improved by your better variable
names, code cleanups should be independent of functionality work.
Minor points:
- In UnitLabel you are deleting the unused Logger. Logging comes and
goes:-), so we have a convention if leaving unused loggers around, they
usually get used again soon enough.
- In DragListener:
-System.err.println("DragListener did not recognize:" + comp);
+logger.log(Level.WARNING, () -> "DragListener did not
recognize:" + comp);
Oops, the System.err.println is a leftover debug trace that I forgot to
remove. I am curious though, why do you use lambda's in loggger
messages?
- Apparently you do not share my appreciation for the ternary operator:-)
Cheers,
Mike Pope
pgpm5qcqrxG9t.pgp
Description: OpenPGP digital signature
___
Freecol-developers mailing list
Freecol-developers@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/freecol-developers
