[Freeciv-Dev] [bug #19447] Over maximum veteran level assigned initially to unit
Update of bug #19447 (project freeciv): Status: Ready For Test = Fixed Open/Closed:Open = Closed ___ Reply to this item at: http://gna.org/bugs/?19447 ___ 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] [bug #19447] Over maximum veteran level assigned initially to unit
Follow-up Comment #10, bug #19447 (project freeciv): Patch looks sensible and I've verified it fixes the problem, thanks. ___ Reply to this item at: http://gna.org/bugs/?19447 ___ 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] [bug #19447] Over maximum veteran level assigned initially to unit
Follow-up Comment #8, bug #19447 (project freeciv): I agree that (2) should be safe. FWIW, I think the unused veteran level fields in RULESET_UNIT packets are initialised to zeroes, by virtue of the implicit zero initialisation of unittype.c:unit_type[] on the server. ___ Reply to this item at: http://gna.org/bugs/?19447 ___ 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] [bug #19447] Over maximum veteran level assigned initially to unit
Update of bug #19447 (project freeciv): Status: In Progress = Ready For Test ___ Follow-up Comment #9: 2) Change client u-veteran_levels = 0 to u-veteran_levels = MAX_VET_LEVELS Untested patch to do just that. (file #15199) ___ Additional Item Attachment: File name: MaxVetLvlToMax.diffSize:0 KB ___ Reply to this item at: http://gna.org/bugs/?19447 ___ 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] [bug #19447] Over maximum veteran level assigned initially to unit
Update of bug #19447 (project freeciv): Status: Fixed = In Progress Open/Closed: Closed = Open ___ Follow-up Comment #7: Well, then I think we should either: 1) Revert this fix from S2_3 This would be safe in the sence that it would certainly not introduce new bugs relative to 2.3.1. 2) Change client u-veteran_levels = 0 to u-veteran_levels = MAX_VET_LEVELS If client does care about this number anywhere but in creation of new units, this change would be safe to do. Together with already committed change it would provide at least some safety to the system. ___ Reply to this item at: http://gna.org/bugs/?19447 ___ 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] [bug #19447] Over maximum veteran level assigned initially to unit
Follow-up Comment #4, bug #19447 (project freeciv): Unfortunately this patch causes a variety of breakage on S2_3 on the client: * Where veteran level names should be displayed for individual units (middle-click popit and city dialog), garbage is displayed instead. * Middle-click popit doesn't show ongoing activity (guessing because client thinks veterancy-dependent activity rate is 0/turn?) * Veteran chevrons not displayed on units. * Units with less than full HP have a full-HP sprite displayed behind the correct one. (Perhaps this is from an out-of-bounds access to the array of veterancy sprites?) I think this is because the client doesn't know the number of veteran levels -- it's not formally communicated in the 2.3 network protocol. From handle_ruleset_unit(): u-veteran_levels = 0; /* not used in the client */ Thus every unit on the client ends up with veteran level -1. We could: * Conditionalise the new check on is_server(). * Roll back the check entirely on S2_3. (With luck, after the fix for bug #19466 it should be irrelevant anyway?) (None of this applies to S2_4 or trunk.) ___ Reply to this item at: http://gna.org/bugs/?19447 ___ 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] [bug #19447] Over maximum veteran level assigned initially to unit
Follow-up Comment #5, bug #19447 (project freeciv): Could client determine number of veteran levels from actual veteran levels information even if the number itself is not explicitly part of the protocol? Information of effects of different veteran levels is certainly needed (move_rate of units comes to mind) ___ Reply to this item at: http://gna.org/bugs/?19447 ___ 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] [bug #19447] Over maximum veteran level assigned initially to unit
Follow-up Comment #6, bug #19447 (project freeciv): I think the client just trusts that UNIT_INFO.veteran is correct and indexes using that. On a quick look I didn't convince myself that unused levels' info in RULESET_UNIT packets were even initialised, so I doubt there are clues there either. ___ Reply to this item at: http://gna.org/bugs/?19447 ___ 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] [bug #19447] Over maximum veteran level assigned initially to unit
Update of bug #19447 (project freeciv): Status: Ready For Test = Fixed Assigned to:None = cazfi Open/Closed:Open = Closed ___ Reply to this item at: http://gna.org/bugs/?19447 ___ 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] [bug #19447] Over maximum veteran level assigned initially to unit
URL: http://gna.org/bugs/?19447 Summary: Over maximum veteran level assigned initially to unit Project: Freeciv Submitted by: cazfi Submitted on: Tue 14 Feb 2012 11:56:36 PM EET Category: general Severity: 3 - Normal Priority: 5 - Normal Status: Ready For Test Assigned to: None Originator Email: Open/Closed: Open Release: Discussion Lock: Any Operating System: None Planned Release: 2.3.2, 2.4.0, 2.5.0 ___ Details: create_unit_virtual() can get called with initial veteran level that exceeds maximum for unit type. It then happily assigns that value to created unit. Attached fix limits veterancy level to maximum for unit type inside create_unit_virtual(). Similar check inside callers would be no more correct than this, and by placing it to create_unit_virtual() all callers are handled at once. I think this bug could cause problems in S2_3 only with ruleset that has only single veterancy level, but has some Veteran_Build -effect. In S2_4 and TRUNK this is more serious problem as Veteran_Build value means number of levels and thus multiple effects together can increase level unexpectedly, and effect that gives just maximum level for one unit type can be over maximum for another unit type. ___ File Attachments: --- Date: Tue 14 Feb 2012 11:56:36 PM EET Name: MaxVetLvlsForVunit.diff Size: 909B By: cazfi http://gna.org/bugs/download.php?file_id=15047 --- Date: Tue 14 Feb 2012 11:56:36 PM EET Name: MaxVetLvlsForVunit-S2_3.diff Size: 903B By: cazfi http://gna.org/bugs/download.php?file_id=15048 ___ Reply to this item at: http://gna.org/bugs/?19447 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
