[Freeciv-Dev] [patch #3829] Allow compatible roads
Update of patch #3829 (project freeciv): Status: Ready For Test = Done Open/Closed:Open = Closed ___ Follow-up Comment #33: For handling autosettlers, patch #4639 opened. ___ Reply to this item at: http://gna.org/patch/?3829 ___ 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 #3829] Allow compatible roads
Update of patch #3829 (project freeciv): Status: In Progress = Ready For Test Assigned to:None = cazfi ___ Reply to this item at: http://gna.org/patch/?3829 ___ 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 #3829] Allow compatible roads
Follow-up Comment #30, patch #3829 (project freeciv): - There's at least one if( instead of if ( - As integrators list is set up on both server and client, should it be function in common code? ___ Reply to this item at: http://gna.org/patch/?3829 ___ 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 #3829] Allow compatible roads
Follow-up Comment #31, patch #3829 (project freeciv): Looking through what this touched, there's lots of if( in trunk: would a cleanup patch be useful? Only fixed the one case where one was being added, rather than anywhere else. I put the integrators cache initialisation into a common function. This requires an extra iteration over roads on ruleset read, so is a bit slower (not that ruleset read is a meaningful optimisation target), and is more lines of code overall. That said, it should now be easier to make changes to this function without needing to coordinate the behaviour in two places. (file #20422) ___ Additional Item Attachment: File name: allow-compatible-roads+common-cache.patch Size:18 KB ___ Reply to this item at: http://gna.org/patch/?3829 ___ 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 #3829] Allow compatible roads
Follow-up Comment #29, patch #3829 (project freeciv): Apologies for taking so long to rebase this. I've kept the integrators vector in roads, rather than extras, mostly because I'm not at all sure how we might want to handle the idea of a road integrating with a base in terms of movement costs, etc. Note that this includes a partial revert of r23610 to support road_type_list_iterate and friends. This is rebased against r24699, and I've removed all the references to bug #16383 (patch #3897 had been applied during the overlong delay). (file #20394) ___ Additional Item Attachment: File name: allow-compatible-roads+svn24699.patch Size:17 KB ___ Reply to this item at: http://gna.org/patch/?3829 ___ 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 #3829] Allow compatible roads
Update of patch #3829 (project freeciv): Status: Ready For Test = In Progress Assigned to: cazfi = None ___ Reply to this item at: http://gna.org/patch/?3829 ___ 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 #3829] Allow compatible roads
Follow-up Comment #28, patch #3829 (project freeciv): the updated patch applies over patch #3886, patch #3897, and a rebased bug #16383, as I consider all those issues to be more important than this (or at least more bug-like and less feature like): I'm happy to rearrange the order if that is preferred for application to trunk. patch #3886 has been applied, but others seem to be halted, or proceeding only slowly, at the moment. Maybe we should proceed with this one first - I'd like to have this feature as base for further development. ___ Reply to this item at: http://gna.org/patch/?3829 ___ 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 #3829] Allow compatible roads
Follow-up Comment #27, patch #3829 (project freeciv): Update to apply over SVN 22834 (and previously mentioned stacking patches). (file #17917) ___ Additional Item Attachment: File name: allow-compatible-roads+svn22834.patch Size:18 KB ___ Reply to this item at: http://gna.org/patch/?3829 ___ 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 #3829] Allow compatible roads
Follow-up Comment #26, patch #3829 (project freeciv): That's the easiest way to implement 12, and what was present already in the patch. Ruleset sanity checking added to ensure that road integration is commutative. For 3, I'll stick with move_cost being determined by the cost of the fastest integrating road on the destination, as it is already implemented that way. This means the following rules apply for move costs (ignoring nativity): a) If there are no roads, move_cost is determined by the destination. b) If there is a road at the destination, the move cost of this road may be used iff there is an integrating road at the source. This replicates current behaviour with comment-only ruleset changes, and allows ruleset authors to choose to allow charging Railroad cost for moves from Road if they desire by causing the roads to integrate (changing the behaviour of condition (b) above). Very ambitious ruleset authors could even define a null road with RMM_NO_BONUS, use lua to place it everywhere on the map, and have it integrate with everything, so that the destination tile always controlled movement entirely. (the RMM_NO_BONUS conditional was moved for this revision of the patch to enable just this (and avoid asserts when a bonus road integrates with a no bonus road: apparently some of (4))). I've been changing tile_move_cost_ptrs() in many places lately: for my convenience, the updated patch applies over patch #3886, patch #3897, and a rebased bug #16383, as I consider all those issues to be more important than this (or at least more bug-like and less feature-like): I'm happy to rearrange the order if that is preferred for application to trunk. Note that bug #16383 does not need changes to unit_move_to_tile_test() for road integration unless there is a decision to impose a static cost for moves between tiles made native by differing roads, in which case integrated roads should not be considered differing roads. (file #17903) ___ Additional Item Attachment: File name: allow-compatible-roads+rssanity.patch Size:18 KB ___ Reply to this item at: http://gna.org/patch/?3829 ___ 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 #3829] Allow compatible roads
Follow-up Comment #25, patch #3829 (project freeciv): 1) Should integrated roads be commutative for movement cost? 2) Should integrated roads be commutative for sprite drawing? 3) What move_cost should be used for integrated roads? For 1 2 I think we may have support for non-commutative behavior in low-level code, but until we see real need (important use-case) for such a setup, we force rulesets to have them commutative (sanity check that when A is integrated to B, also B is integrated to A). For 3, we don't seem to have any gameplay related argument to base our decision to, so it's probably going to be what's easiest to implement (comes most naturally out of the code / takes least CPU cycles) - if we know by first part already that speed with these integrated roads is not going to be better than with currently selected one, we don't need to iterate through potentially integrating roads. ___ Reply to this item at: http://gna.org/patch/?3829 ___ 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 #3829] Allow compatible roads
Follow-up Comment #20, patch #3829 (project freeciv): One thing that I've always (since first civ1 game two decades ago) found a bit inconsistent is what movement cost gets used. 1) If there's no road in target tile, it's movement cost is used. It doesn't matter if first half of your journey is climing down from another mountain (moving from mountain to mountain tile), or train trip on plains (plains with railroad to mountain), the cost is the same 2) If there's road in target tile, the source tile suddenly matters. If there's no road in source tile, target road is not used. (Also, if there's railroad (+ road) on target, but only road on source, road cost is used) Now I'm not entirely sure which cost should apply to integrated roads movement. Probably the slower of the two, be it in source or target tile, to be consistent with existing railroad (+implicit road) in one tile, road in another -model. Do you have some specific reason this should be in 2.5? (If you plan to publish high-quality ruleset that uses this for 2.5, we can consider including this, but otherwise, given number of things to resolve, possibly including some AI changes, I'd rather target this to 2.6) ___ Reply to this item at: http://gna.org/patch/?3829 ___ 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 #3829] Allow compatible roads
Follow-up Comment #21, patch #3829 (project freeciv):
This was only in my 2.5 list because I thought I would be able to write my
ruleset for 2.5. Given that the ruleset I want to write has no units that are
either native to all land tiles or all water tiles, and many of the air
units cannot reach all tiles, there's far too much nativity work to do to the
engine for me to expect anything to work (I was seduced by some of the content
in NEWS and scattered bits in the SVN logs moving towards 2.5). Additionally,
I needed the BaseFlag/RoadFlag stuff, which has already been pushed to 2.6.
I'm not sure that my ruleset would be high quality when first published
anyway: there are far too many possible balancing issues that need to be
worked out, and I still have some conceptual issues to resolve (e.g. Should
Neritic units be native to Lakes without dredging? What combination of
combat_bonus entries best models depth charges for better rock/paper/scissors
interaction of units? Should borax be a bonus resource without a strip mine?
If not, how can the AI be convinced not to transform the tile?) anyway.
On movement, I think that for compatibility, it makes sense to just
arbitrarily choose one of source, destination, minimum, or maximum cost to use
for integrated roads (and I'm not sure which either). For future direction,
it probably makes sense to use some general game parameter that chooses from
different move modes, and then either set a callback for different
implementations of tile_move_cost_ptrs (or portions thereof), or find a
low-cost way of considering multiple solutions in the same implementation. I
don't think that longer-term plan belongs as part of this ticket, as it has
wider scope, really, but rather that we ought just pick something and use that
(potentially including the slowest of the mixed roads).
Another thing I realise reviewing this ticket is that I never answered the
question about sprites and integration. At least for the tileset I use, there
are separate sprites for each direction, which are all layered into the final
image. In the case where road A integrates with road B, but road B does not
integrate with road A, and tile X has road A and tile Y has road B, the
current code will draw a connector for road A on tile X towards tile Y, but
not draw a connector from tile Y towards tile X (for either road type), which
looks a bit odd if we gain the movement benefit of road B (but may provide the
right visual feedback if we select another move_cost to use).
And as an aside, with this and roadflags, we can drop TF_BRIDGE,
RF_REQUIRES_BRIDGE and potentially RF_PREVENTS_OTHER_ROADS, because we'll be
able to describe the behaviour associated with those in terms of integrates[]
and reqs{} (and those with interest in making everything very pretty can then
even have different graphics in place for bridges, if they so desire).
___
Reply to this item at:
http://gna.org/patch/?3829
___
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 #3829] Allow compatible roads
Follow-up Comment #22, patch #3829 (project freeciv): Thinking about it, what AI changes would be related to this? Autosettler road selection for route building? Pathfinding should already be handled due to the implementation of single_move_cost(), and traditional ferryboats don't get road benefits (so aiferry doesn't really care: more because it needs so much work than because it shouldn't care, but that's a different issue). Not that this is an argument for inclusion in 2.5, but rather that I would prefer not to leave something undone related to this, especially the AI, as I tend to play single-player and it annoys me when the AI is unable to deal with something (for example, with the classic ruleset, I have found it convenient to use paratroopers and stealth bombers to win the game before building boats, because then the AI thinks it doesn't need to worry about defense, making it easy to conquer everything). I still can't beat the AI at land vs. land without a technical or massive production advantage, so I know that when I win it is usually because the AI just didn't understand the strategic or tactical options available with the ruleset. ___ Reply to this item at: http://gna.org/patch/?3829 ___ 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 #3829] Allow compatible roads
Update of patch #3829 (project freeciv): Planned Release: 2.5.0 = 2.6.0 ___ Follow-up Comment #23: I was thinking about autosettler. But at the same time, based on my in-game observations, it seems that AI is even now never building roads for connectivity reasons (whether there's some hard bug, or is the want just too low to ever get priority, I don't know). Road building seems to be completely driven by the tile bonuses they give. ___ Reply to this item at: http://gna.org/patch/?3829 ___ 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 #3829] Allow compatible roads
Follow-up Comment #24, patch #3829 (project freeciv): I think it's a combination of the want being small and there being some bugs with the magic numbers in adv_settlers_road_bonus being wrong (at least I think they are wrong: so far I only understand about 70% of the code under the if you can understand the algorithm below please rewrite this explanation comment (2 definitely and one almost), but that's not quite enough to be certain about changing the numbers). Fixing that would be good, and fixing it with integrated roads even better, but I'm not sure I like the idea of stalling this waiting for that, although I don't have any issues if that happens to be ready before this coincidentally. As I see it, there are four remaining things to determine for this ticket: 1) Should integrated roads be commutative for movement cost? 2) Should integrated roads be commutative for sprite drawing? 3) What move_cost should be used for integrated roads? 4) Are there any more outstanding masked bugs in the basic implementation (like the one found about the client not actually receiving the bitvector)? The first three aren't especially easy questions, and the last will only be resolved with more testing (especially as the patch is updated for the first three issues), so that I think there aren't many left isn't meant as an indicator that the patch is ready or the ticket is almost done. ___ Reply to this item at: http://gna.org/patch/?3829 ___ 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 #3829] Allow compatible roads
Follow-up Comment #10, patch #3829 (project freeciv): How are integrated roads working visually? There seems to be no tilespec.c changes in the patch. I would expect something similar to ConnectLand flag handling. ___ Reply to this item at: http://gna.org/patch/?3829 ___ 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 #3829] Allow compatible roads
Follow-up Comment #11, patch #3829 (project freeciv): There was no special display, which meant sometimes one could travel easily between tiles that did not appear to be connected. Attached patch should cause roads to be drawn as connected to any adjacent integrating roads (respecting cardinality). Do you have a suggested test case for this? The code makes sense to me, and seems to follow the general ideas other places in fill_road_sprite_array(), but I'm not thinking of how to construct something that lets me see the arrangement to be sure (without a suggestion, I'll revisit this after sleeping). (file #17830) ___ Additional Item Attachment: File name: allow-compatible-roads+tilespec.patch Size:16 KB ___ Reply to this item at: http://gna.org/patch/?3829 ___ 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 #3829] Allow compatible roads
Follow-up Comment #12, patch #3829 (project freeciv): Please ignore that last post: the attached patch doesn't compile, and the test case becomes obvious with a clear head. Apologies for the noise. ___ Reply to this item at: http://gna.org/patch/?3829 ___ 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 #3829] Allow compatible roads
Follow-up Comment #13, patch #3829 (project freeciv): So, previously the client didn't do anything graphically, and this ended up masking a bug that the client didn't actually *know* which roads integrated (proad-integrates was always an empty bitvector, rather than being read from the packet) [prior testing worked for simple moves, because the server would correctly set the moves_left value, but was suboptimal for client-side pathfinding calculations). New patch properly draws connections between integrated roads (well, from a road towards roads it integrates with: roads integrated with won't extend to integrating roads (doing this requires setting both roads as cross-integrators in the ruleset)). During testing of this change, I began to wonder if this patch should care about commutativity. For graphical display, I'd argue it should not care (because we don't want to draw random spurs from a high-speed road to low-speed roads just because they are there). For tile_move_cost_ptrs, perhaps it would be useful (because it doesn't make sense to pay full non-road penalties when changing roads in one direction but not the other). Alternately, figuring out how to handle commutativity for movement vs. display might be just as well deferred to something else, so that others also get more playtesting with this implementation to develop an opinion. (file #17833) ___ Additional Item Attachment: File name: allow-compatible-roads+client.patch Size:17 KB ___ Reply to this item at: http://gna.org/patch/?3829 ___ 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 #3829] Allow compatible roads
Follow-up Comment #15, patch #3829 (project freeciv): My graphical test was trivial: define some integrated roads in a ruleset, using existing graphics. Start a new game. Add some roads in the editor. There's a separate issue that sometimes sprites don't refresh until there is a new cause of vision for a tile, but that's independent of this patch (and is only likely to be noticed playing with road drawing and connectivity in the editor). Now, about commutativity: currently each road has a separate integrates vector, and so it is possible to construct a road set where road A integrates with road B, but road B does not integrate with road A. The consequences of such configuration are as follows: 1) Move from tile with (only) road A to tile with (only) road B costs B.move_cost movement points. 2) Move from tile with (only) road B to tile with (only) road A costs (is_igter ? MOVE_COST_IGTER : terrain.movement_cost*SINGLE_MOVE) movement points. 3) When drawing a sprite for a tile with (only) road A, if an adjacent tile has (only) road B, a road A type connector will be drawn towards that tile, if the tileset supports this. 4) When drawing a sprite for a tile with (only) road B, if an adjacent tile has (only) road A, a road B connector will not be drawn towards that tile (nor a road A connector), regardless of tileset support. My initial thought is that case 2) represents a bug, in that the cost ought to be A.move_cost movement points, although thinking about it more, someone (not me) might want to model disembarking from a train with such an additional movement penalty. That said, I think case 4) represents a feature: it probably looks better to draw some roads (e.g. rail) as not having spur connections to feeders (e.g. road), without requiring the integrating road to be present on the rail square and using hiders. This may be an incomplete feature, as it may be interesting to draw spur sprites for incoming integrating roads. Also, it gets hard to determine when to do/not do this, so perhaps integrating the draw logic with hiders is better, not drawing a spur (road_near[dir] == FALSE) when the road being drawn would hide the road that is potentially near (in which case 4) is neither a bug nor a feature, but irrelevant). Implementation of this means either carrying two cache vectors client side or modifying the sprite logic to use binary operators on the road_near[dir] and troad.integrates bitvectors rather than an iteration loop (which I like better, and should be faster, but actually doing it requires more review of syntax than the other way). Regardless of the solution chosen for sprites, one would just cross-populate the integrators lists for all integrating roads, before pushing them through the sort and unique functions (which probably move somewhere else, so they can be processed once ruleset load is complete). The above notwithstanding, if we don't care about commutativity, there's nothing to be done, and we have behaviours 1-4 all working. ___ Reply to this item at: http://gna.org/patch/?3829 ___ 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 #3829] Allow compatible roads
Follow-up Comment #16, patch #3829 (project freeciv): Err, failed to answer the independence question. For my testing, the roads neither depended upon each other nor conflicted with each other, so I could test the results with overlapping and non-overlapping roads. ___ Reply to this item at: http://gna.org/patch/?3829 ___ 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 #3829] Allow compatible roads
Follow-up Comment #17, patch #3829 (project freeciv): Just in case anyone else happens to think BV_CHECK_MASK would be faster than tracking cached lists, it's not: although it performs slightly fewer operations per cycle, it also performs operations for all empty bits, which doesn't help. That said, someone else might have a better understanding of utility/bitvector.[ch] than I, in which case, I encourage them to use their judgement. ___ Reply to this item at: http://gna.org/patch/?3829 ___ 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 #3829] Allow compatible roads
Follow-up Comment #18, patch #3829 (project freeciv): ...and vast majority of the bits are empty ones in all existing rulesets. e.g. alien ruleset has 7 roads of which every one connects with itself only (1 bit out of 7 set - or would it be even 1 out of MAX_NUM_ROADS bits?). ___ Reply to this item at: http://gna.org/patch/?3829 ___ 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 #3829] Allow compatible roads
Follow-up Comment #19, patch #3829 (project freeciv): Looks to me like MAX_ROAD_TYPES (based on my reading of the definition of BV_DEFINE), but as it is early-termination, this may or may not be horrible (depending on the road being checked). That said, since we tend to sort roads in rulesets from slowest to fastest, the roads that are most interesting to check are checked last (whereas with the sorted integrators vectors, they are checked first). Probably needs some thought, but it might be interesting to sort hiders, if there is a sensible sort field, for the same class of early-termination optimisation. Anyway, none of this particularly matters except in terms of which possible method might be used if we wanted to have differing commutative behaviour for integrated roads for graphical placement and movement cost, and I don't think there's even consensus on whether road integration should be commutative yet :) ___ Reply to this item at: http://gna.org/patch/?3829 ___ 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 #3829] Allow compatible roads
Update of patch #3829 (project freeciv): Status:None = Ready For Test Assigned to:None = cazfi Planned Release: = 2.5.0 ___ Reply to this item at: http://gna.org/patch/?3829 ___ 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 #3829] Allow compatible roads
Follow-up Comment #8, patch #3829 (project freeciv): Now I also know what I failed to see last week - and still do. While road is added to the list, it's not added to the bitvector. As bitvetor is currently used only for transmitting information to client side, I assume everything to work on server side, but client side goto not to consider road integrated to itself. ___ Reply to this item at: http://gna.org/patch/?3829 ___ 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 #3829] Allow compatible roads
Follow-up Comment #9, patch #3829 (project freeciv): This is why the road is also self-added to the integrators road_type_list in packhand.c:handle_rulesets_ready() (as part of the same iteration used to process hiders): by not forcing self in the bitvector, the implementation is entirely separated from the representation. An updated patch is attached that addresses the ordering and uniqueness of integrators. I didn't like any of the ways I ended up trying to implement the mechanism discussed in my last post, so ended up adding comments and using the unique and sort list operators on the list. (file #17772) ___ Additional Item Attachment: File name: allow-compatible-roads+sort-u.patch Size:15 KB ___ Reply to this item at: http://gna.org/patch/?3829 ___ 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 #3829] Allow compatible roads
Follow-up Comment #4, patch #3829 (project freeciv): Err, rather, the code is just confusing: suggestions for comments to avoid confusing future readers welcome. Rulesets add compatible roads to the integrates vector. load_ruleset_terrain(): Adds road being loaded to the (server-side) integrators list Adds any road in the integrates INI vector to the (server-side) integrators list Sets a bitvector to match the INI vector for integrates packets.def: sends the integrates bitvector handle_rulesets_ready(): Adds road being processed to the (client-side) integrators list Adds any roads in the integrates bitvector to the (client-side) integrators list tile_move_cost_ptrs(): Iterates over the integrators list when checking roads. So, for both client and server, every road always integrates with itself, regardless of the notation in the ruleset. Further, the identical road is always first in the integrators list, so that continuation on the same road is checked first when calculating move cost (which is expected to be the most common case, and for fast roads, the lowest cost (important as movement over fast roads requires the greatest number of calls to tile_move_cost_ptrs). Therefore, the originally submitted patch should be fine to process (unless something else is discovered in testing). That said, perhaps integrates and integrators need greater differentiation to reduce confusion, or, as noted above, perhaps the code needs more comments. ___ Reply to this item at: http://gna.org/patch/?3829 ___ 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 #3829] Allow compatible roads
Follow-up Comment #5, patch #3829 (project freeciv): am I blind Yes, I am. I assumed it to work that way, yet I somehow missed the line where it did it. regardless of the notation in the ruleset. Oh, but if ruleset *does* have road type itself (redundantly) listed, it will be added twice to the list, no? Or generally: any road can be added multiple times to the server side list. This should be easy to check from the bitvector side. If the bit is already set, do not add road again. ___ Reply to this item at: http://gna.org/patch/?3829 ___ 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 #3829] Allow compatible roads
Follow-up Comment #6, patch #3829 (project freeciv): Further, the identical road is always first in the integrators list, so that continuation on the same road is checked first when calculating move cost (which is expected to be the most common case, and for fast roads, the lowest cost For shaving cycles it actually might make sense to order roads in the list from lowest cost to highest. tile_move_cost_ptrs() could rely on that order to break iteration as soon as it finds first integrating road. ___ Reply to this item at: http://gna.org/patch/?3829 ___ 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 #3829] Allow compatible roads
Follow-up Comment #7, patch #3829 (project freeciv): Ah, good point. Perhaps rather than adding roads to the integrators list at parse time, it ought just set the bitvector (redundantly setting it causes no issue, and checking that it is set likely usually wastes cycles). Once the iterates bitvector is set, a list can be constructed from it, sorted by cost (lowest to highest), and stored as integrators. Having a separate chunk of code specifically setting integrators should also make the entirety more readable, so that this confusion doesn't arise again (as both of us read it as wrong last week, the current code is clearly suboptimal). I don't think it is safe to terminate the tile_move_cost_ptrs() loop once a road is found, because we can't know that it hasn't hit the RMM_RELAXED condition, but the cost of iterating through the remainder should be low, because we check (cost iroad-move_cost) early enough that the remainder of the iteration should go quickly. ___ Reply to this item at: http://gna.org/patch/?3829 ___ 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 #3829] Allow compatible roads
Follow-up Comment #2, patch #3829 (project freeciv): Not tested yet, but am I blind when I don't see how road type integrates with itself in the code? ___ Reply to this item at: http://gna.org/patch/?3829 ___ 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 #3829] Allow compatible roads
Follow-up Comment #3, patch #3829 (project freeciv): Roads integrate with themselves by forced insertion of the road for which the vector is being constructed into it's own vector immediately following initialisation (and before reviewing the integrates field) in ruleset.c and packhand.c. Or, rather, that was the intent: looking at the code again, I'm not convinced it works that way at all, and I no longer understand how I could have achieved successful test results. I'll reset and review, and very much suspect this will need an updated patch that correctly sets the bitvector on the server side (it gets set on the client side because the network-delivered ruleset doesn't match the actual ruleset, but that working looks to be a par of bugs interacting coincidentally the right way rather than correct code). ___ Reply to this item at: http://gna.org/patch/?3829 ___ 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 #3829] Allow compatible roads
Follow-up Comment #1, patch #3829 (project freeciv): maxiumum number of roads has been increased from 8 to 16. I assume memory consumptionb to remain about the same (alignment adjustments were already wasting the byte now taken to use) Savegames are already dynamically adjusting so that increase in MAX does not cause increase in size of the games not using added slots. ___ Reply to this item at: http://gna.org/patch/?3829 ___ 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 #3829] Allow compatible roads
URL: http://gna.org/patch/?3829 Summary: Allow compatible roads Project: Freeciv Submitted by: persia Submitted on: Wed 03 Apr 2013 11:11:55 AM GMT Category: general Priority: 5 - Normal Status: None Privacy: Public Assigned to: None Originator Email: Open/Closed: Open Discussion Lock: Any Planned Release: ___ Details: This patch allows one to define multiple roads that both behave similarly and allow continuous transit at the provided move_cost between them. Any movement from a tile containing a road to another tile containing a compatible road will entail a move_cost of the destination road: for example, if one defined a Mountain Trail road with a move_cost of 3, which integrated with Road (and Road integrated with Mountain Trail), units native to both Road and Mountain Trail would be able to move from any tile with a Road to a tile with a Mountain Trail at one third the normal cost of entering a Mountain tile (perhaps interesting for mounted troops who normally would be forced to end their turn atop the mountain). Similarly, one would be able to leave the Mountain Trail containing tile and move into a Road tile with only the Road move cost being deducted from the unit movement points. As this offers considerable incentive for ruleset authors to further differentiate roads (more options for roads, easier to have different roads for different purposes, etc.), the maxiumum number of roads has been increased from 8 to 16. Application of this patch will require a capability string change, as the network protocol has changed. This patch expects that patch #3820 has already been applied (and may have other offsets, but no other direct dependencies). ___ File Attachments: --- Date: Wed 03 Apr 2013 11:11:55 AM GMT Name: allow-compatible-roads.patch Size: 14kB By: persia http://gna.org/patch/download.php?file_id=17665 ___ Reply to this item at: http://gna.org/patch/?3829 ___ 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 #3829] Allow compatible roads
Update of patch #3829 (project freeciv): Depends on: = patch #3820 ___ Reply to this item at: http://gna.org/patch/?3829 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
