Project

General

Profile

Actions

Bug #1278

closed

bug in can_reclaim_ocean()

Added by Dean Brown 11 months ago. Updated 1 day ago.

Status:
Closed
Priority:
Normal
Category:
General
Target version:
Start date:
04/01/2025
Due date:
% Done:

0%

Estimated time:

Description

Found while testing #1277, this is an obscure edge case that has likely never happened.

Repro -
civ2civ3 ruleset
edit terrain.ruleset -> ocean_reclaim_requirement = 67
hex tileset
use editing mode to get tech Fusion
put Engineers on a Transport in the ocean on tile with 4 adjacent land tiles
transform to land is available

67% of 6 is 4.02, so should require 5 adjacent land tiles.

  int land_tiles = 100 - count_terrain_class_near_tile(nmap, ptile,
                                                       FALSE, TRUE,
                                                       TC_OCEAN);

count_terrain_class_near_tile() returns 33%, which should really be 33.3%,
100 - that = 67, should really be 66.7,
and 66.7 >= 67 should fail but 67 >= 67 succeeds.

Obvious fix is

  int land_tiles = count_terrain_class_near_tile(nmap, ptile,
                                                 FALSE, TRUE, TC_LAND);

But there must have been some reason it was coded like it is,
instead of the obvious way - wish there were a comment.
The way it is has side effect that Inaccessible and "off-the-map" tiles
are counted as land tiles.


Files

1278_S3_2.patch (1.96 KB) 1278_S3_2.patch Dean Brown, 01/05/2026 12:15 AM
1278.patch (1.95 KB) 1278.patch Dean Brown, 01/28/2026 03:05 AM
Actions #1

Updated by Dean Brown 11 months ago

  • File patch1278.diff added
Actions #2

Updated by Dean Brown 7 months ago

  • File deleted (patch1278.diff)
Actions #3

Updated by Dean Brown 7 months ago

  • File 1278.patch added

renamed the patch

Actions #4

Updated by Marko Lindqvist about 2 months ago

Dean Brown wrote:

But there must have been some reason it was coded like it is,

...

The way it is has side effect that Inaccessible and "off-the-map" tiles
are counted as land tiles.

I think you got the reason just there. Especially on the polar areas (polar strips) we want the off-map area to count as land and not as ocean. Maybe this ticket should be fixed by adding the comment about that?

Actions #5

Updated by Dean Brown about 2 months ago

Yes it needs a comment. Also needed to do things differently to fix the bug.

Actions #6

Updated by Dean Brown about 2 months ago

  • File deleted (1278.patch)
Actions #7

Updated by Dean Brown about 2 months ago

  • File 1278_S3_2.patch added
  • File 1278.patch added

1278.patch for S3_3 and Main.

Actions #8

Updated by Marko Lindqvist about 1 month ago

- "HR #1278" -> "RM #1278"

Actions #9

Updated by Dean Brown about 1 month ago

  • File deleted (1278_S3_2.patch)
Actions #10

Updated by Dean Brown about 1 month ago

  • File deleted (1278.patch)
Actions #11

Updated by Dean Brown about 1 month ago

  • File 1278_S3_2.patch added
  • File 1278.patch added

fixed

Actions #12

Updated by Marko Lindqvist about 1 month ago

The comment still says "HR #1278" (I assume you fixed commit message instead)

Actions #13

Updated by Dean Brown about 1 month ago

  • File deleted (1278_S3_2.patch)
Actions #14

Updated by Dean Brown about 1 month ago

  • File deleted (1278.patch)
Actions #15

Updated by Dean Brown about 1 month ago

fixed

Actions #16

Updated by Marko Lindqvist 17 days ago

In main and S3_3 you should use MAP_NUM_VALID_DIRS instead of wld.map.num_valid_dirs.

Actions #17

Updated by Dean Brown 17 days ago

Done

Actions #18

Updated by Dean Brown 17 days ago

  • File deleted (1278.patch)
Actions #19

Updated by Marko Lindqvist 13 days ago

  • Status changed from New to In Review
  • Assignee set to Marko Lindqvist
  • Target version set to 3.2.3
Actions #20

Updated by Marko Lindqvist 1 day ago

  • Status changed from In Review to Closed
Actions

Also available in: Atom PDF