[Freeciv-Dev] [patch #5367] Consider move_rate when picking a transport in base_transporter_for_unit()
Follow-up Comment #10, patch #5367 (project freeciv): In the case you compute all values at the beginning of the unit process My patch is a hybrid; it only hoists the 'expensive' calculations to the top, to avoid iterating over the transport stack multiple times. Cheap data collection happens next to the relevant test. So it's still necessary to structure the code as you had it. I'm assuming this isn't an objection to commit so will commit this now, as we've probably spent too long on it as it is. I'll revisit it if requested. ___ Reply to this item at: http://gna.org/patch/?5367 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #5367] Consider move_rate when picking a transport in base_transporter_for_unit()
Update of patch #5367 (project freeciv): Status: Ready For Test = Done Open/Closed:Open = Closed ___ Reply to this item at: http://gna.org/patch/?5367 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #5367] Consider move_rate when picking a transport in base_transporter_for_unit()
Follow-up Comment #9, patch #5367 (project freeciv): In the case you compute all values at the beginning of the unit process, there would be probably no reason to keep 'best_trans != ptrans' tests and fallback to 'best = cur'. I would suggest then to replace every 'best_trans = ptrans' occurence by: best_trans = ptrans; best = cur; continue; ___ Reply to this item at: http://gna.org/patch/?5367 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #5367] Consider move_rate when picking a transport in base_transporter_for_unit()
Follow-up Comment #8, patch #5367 (project freeciv): New patches taking into account pepeto's restructure, and adding a tie break at the end taking into account all transports' moves_left and move_rate. (file #22747, file #22748) ___ Additional Item Attachment: File name: trunk-choose-transport-4.patch Size:5 KB File name: S2_5-choose-transport-4.patch Size:5 KB ___ Reply to this item at: http://gna.org/patch/?5367 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #5367] Consider move_rate when picking a transport in base_transporter_for_unit()
Follow-up Comment #5, patch #5367 (project freeciv): Feel free. This could looks like this... Also: * noticed it considers the moves points left of the units, not the move rate. This could probably be improved; * I think a test with the sum of all transporters' move rate(/moves left) may be used for a last test. (file #22673) ___ Additional Item Attachment: File name: base_transporter_for_unit.patch Size:4 KB ___ Reply to this item at: http://gna.org/patch/?5367 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #5367] Consider move_rate when picking a transport in base_transporter_for_unit()
Follow-up Comment #6, patch #5367 (project freeciv): This could looks like this... It's verbose, but at least more conditions can be added with it disappearing off the right margin. OK. noticed it considers the moves points left of the units, not the move rate. This could probably be improved; Of course this function has no idea if the cargo is setting out on a short journey or a long one. I think moves left should have higher priority; the scenario I have in mind is two transports in a city, one of which has moved this turn; I want to choose the other. That's why I chose moves_left. While we could sum moves_left and moves_rate the result could be counterintuitive -- don't want to force players to do arithmetic to work out what their UI action will do -- so probably move_rate should be a lower term in the lexicographic test, below moves_left which players can see directly in the UI. I think a test with the sum of all transporters' move rate(/moves left) may be used for a last test. Could do, as we're iterating over transports anyway. Perhaps combine with the previous: sum all moves_left + move_rate (which for the common case of single levels of transport degenerates to test moves_left, then test moves_left+move_rate, which is like testing moves_left then move_rate). ___ Reply to this item at: http://gna.org/patch/?5367 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #5367] Consider move_rate when picking a transport in base_transporter_for_unit()
Follow-up Comment #7, patch #5367 (project freeciv): Actually I didn't think about moves left+move rate, but about a sum of moves left or a sum of move rate... ___ Reply to this item at: http://gna.org/patch/?5367 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #5367] Consider move_rate when picking a transport in base_transporter_for_unit()
Update of patch #5367 (project freeciv): Status:None = Ready For Test Assigned to: pepeto = jtn ___ Follow-up Comment #2: Darn, I wrote this patch before noticing that pepeto had taken the ticket. pepeto, feel free to steal it back for further work. From the comments, these are the new criteria as of this patch (2, 5, and 6 are my new conditions): 0 Transports which have orders, or are on transports with orders, are less preferable to transport stacks without orders (to avoid loading on units that are just passing through); else 0 Transports which are idle are preferable (giving players some control over loading) -- this does not check transports of transports; else 0 Transports from which the cargo could unload at any time are preferable to those where the cargo can only disembark in cities/bases; else 0 Transports which are less deeply nested are preferable; else 0 Transport stacks where the outermost transport has more moves left are preferable (if we're loading now, presumably it's the outermost transport that's about to move); else 0 Direct transports with more moves left are preferable. Of these (2) might be the most controversial. I thought it was worth giving users control over which transport was chosen (as I said in patch #4982, but I was wrong that it was already possible). Note that this doesn't stop automatic pathfinding using sentried pontoon bridges etc. Also: I think that as of patch #4982, the lexicographic ordering was not as intended; depending on the order of the units on the tile, I think depth might trump has_orders, which I don't think was intended. Attached patch also fixes this, but the code is quite ugly. I couldn't think of a nice idiomatic way to express this kind of comparison in C. (file #22666, file #22667) ___ Additional Item Attachment: File name: trunk-choose-transport-3.patch Size:4 KB File name: S2_5-choose-transport-3.patch Size:4 KB ___ Reply to this item at: http://gna.org/patch/?5367 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #5367] Consider move_rate when picking a transport in base_transporter_for_unit()
Follow-up Comment #3, patch #5367 (project freeciv): 2. Idle units should maybe include sentried, fortifying, or fortified ones? The C condition looks very ugly. I have some idea to improve it (if I have time before you commit your patch). ___ Reply to this item at: http://gna.org/patch/?5367 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #5367] Consider move_rate when picking a transport in base_transporter_for_unit()
Follow-up Comment #4, patch #5367 (project freeciv): 2. Idle units should maybe include sentried, fortifying, or fortified ones? I explicitly didn't include these, so that users can override the automatic selection by putting the transport they didn't want chosen into sentry mode. Yes, this is a UI hack (compensating for the lack of bug #13943), and will probably confuse someone, but without it the only ways to override unwanted transport selection probably involve spending movement points; whereas if you wanted to load onto a fortified transport in the presence of an idle one (and for some reason you couldn't sentry/fortify the unwanted one), making the fortified unit temporarily idle for loading then fortifying again doesn't lose fortified status. The C condition looks very ugly. I have some idea to improve it (if I have time before you commit your patch). Feel free. I'll hold off committing. It is very ugly. ___ Reply to this item at: http://gna.org/patch/?5367 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #5367] Consider move_rate when picking a transport in base_transporter_for_unit()
Update of patch #5367 (project freeciv): Assigned to:None = pepeto ___ Reply to this item at: http://gna.org/patch/?5367 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #5367] Consider move_rate when picking a transport in base_transporter_for_unit()
Update of patch #5367 (project freeciv): Summary: Consider move_rate when picking a transport in transporter_for_unit() = Consider move_rate when picking a transport in base_transporter_for_unit() ___ Reply to this item at: http://gna.org/patch/?5367 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
