Re: [Freeciv-Dev] (PR#39330) Re: (PR#39335) Re: Fix for PR#39330
http://bugs.freeciv.org/Ticket/Display.html?id=39330 > On 4/11/07, [EMAIL PROTECTED] <[EMAIL PROTECTED]> wrote: > > http://bugs.freeciv.org/Ticket/Display.html?id=39330 > > > Jason Short wrote: > > http://bugs.freeciv.org/Ticket/Display.html?id=39330 > > > >> Well, the patch can be easily be modified to send 'TILE_UNKNOWN' > >> instead, for the case when the tile is seen by a city, but unknown. > >> A *seen* tile is allowed to be *unknown* according to a comment in > >> server/maphand.c, starting at line 810 on the SVN head revision, which I > >> quote here: > > > > A tile can be known but not seen. In this case the tile should be > > marked as TILE_UNKNOWN in the player tile, NOT as TILE_KNOWN_FOGGED. > > No, a known but unseen tile *should* be marked as TILE_KNOWN_FOGGED. The > code already does this, and my patch does not change this. Very sorry, I mispoke. I meant to say, a seen but unknown tile is valid and should be marked as TILE_UNKNOWN. This is what is happening, is it not? > Anyway, I modified the patch as I said I would, so that unknown but seen > tiles are marked as TILE_UNKNOWN now. I tested it, and it works fine. > The revised patch is attached to this message. But we're still not fixing the core problem which is that an unknown tile is being marked (in the plrtile) as KNOWN_FOGGED. -jason ___ Freeciv-dev mailing list [EMAIL PROTECTED] https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39330) Re: (PR#39335) Re: Fix for PR#39330
http://bugs.freeciv.org/Ticket/Display.html?id=39330 >
Jason Short wrote:
> http://bugs.freeciv.org/Ticket/Display.html?id=39330 >
>> Well, the patch can be easily be modified to send 'TILE_UNKNOWN'
>> instead, for the case when the tile is seen by a city, but unknown.
>> A *seen* tile is allowed to be *unknown* according to a comment in
>> server/maphand.c, starting at line 810 on the SVN head revision, which I
>> quote here:
>
> A tile can be known but not seen. In this case the tile should be
> marked as TILE_UNKNOWN in the player tile, NOT as TILE_KNOWN_FOGGED.
No, a known but unseen tile *should* be marked as TILE_KNOWN_FOGGED. The
code already does this, and my patch does not change this.
Anyway, I modified the patch as I said I would, so that unknown but seen
tiles are marked as TILE_UNKNOWN now. I tested it, and it works fine.
The revised patch is attached to this message.
> What is a bit tricky is that map_get_seen can
> return TRUE for unknown tiles
That is precisely the case the patch deals with, since it wasn't being
dealt with before.
P.S. Send future reports to the bug tracker.
>> I *did* send the *report* to the tracker. Are you asking me to only send
>> future *patches* for open/new tickets to the tracker and not the list?
>> Do you want me to create a new ticket for each patch, like you
>> apparently did to respond to my earlier message?
>
> You sent the report to the tracker then sent the patch to the list.
Part of the reason for this was to make sure that I could actually post
to the list, if needed. :-)
Eric
Index: server/maphand.c
===
--- server/maphand.c(revision 12916)
+++ server/maphand.c(working copy)
@@ -475,10 +475,10 @@
&& map_get_seen(ptile, pplayer, V_MAIN) == 0) {
struct player_tile *plrtile = map_get_player_tile(ptile, pplayer);
- info.known = TILE_KNOWN_FOGGED;
- info.type = plrtile->terrain->index;
+ info.known = plrtile->terrain ? TILE_KNOWN_FOGGED : TILE_UNKNOWN;
+ info.type = plrtile->terrain ? plrtile->terrain->index : -1;
for (spe = 0; spe < S_LAST; spe++) {
- info.special[spe] = BV_ISSET(plrtile->special, spe);
+info.special[spe] = BV_ISSET(plrtile->special, spe);
}
info.resource = plrtile->resource ? plrtile->resource->index : -1;
info.continent = ptile->continent;
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39330) Re: (PR#39335) Re: Fix for PR#39330
http://bugs.freeciv.org/Ticket/Display.html?id=39330 > On 4/11/07, [EMAIL PROTECTED] <[EMAIL PROTECTED]> wrote: > > http://bugs.freeciv.org/Ticket/Display.html?id=39330 > > > Jason Short wrote: > > http://bugs.freeciv.org/Ticket/Display.html?id=39335 > > > >>Attached to this message is a patch to fix the issue reported in > >> PR#39330. Basically, the problem seems to be that cities can see unknown > >> terrain, and this is considered acceptable and normal. But, in at least > >> one place in the code that possibility is overlooked. In a player's view > >> of a tile, the terrain ptr may be NULL (indicating it is unknown). > >> Attempts to reference a member, such as 'type', of such a NULL ptr > >> obviously results in a segfault. > > > > Good catch but the bug is deeper. We do NEED to know the terrain type > > here since the tile is being sent as TILE_KNOWN_FOGGED. Can you find > > a way to reproduce the problem and figure out why a "known" tile has > > unknown terrain type? That should not be possible. > > Well, the patch can be easily be modified to send 'TILE_UNKNOWN' > instead, for the case when the tile is seen by a city, but unknown. > A *seen* tile is allowed to be *unknown* according to a comment in > server/maphand.c, starting at line 810 on the SVN head revision, which I > quote here: A tile can be known but not seen. In this case the tile should be marked as TILE_UNKNOWN in the player tile, NOT as TILE_KNOWN_FOGGED. This isn't an error in the network code - it's an error in whatever code marked the plrtile as TILE_KNOWN_FOGGED yet didn't set the terrain. The vision code *has* changed recently so there is a reasonable chance of errors of this type. If we're able to reproduce it I can probably track it down next week, but if you can find the error yourself that would naturally be even better. > / >Return whether the player can see the tile. Seeing a tile means you have >vision of it now (as opposed to knowing a tile which means you've seen it >before). Note that a tile can be seen but not known (currently this only >happens when a city is founded with some unknown tiles in its radius); in >this case the tile is unknown (but map_get_seen will still return TRUE). > / > > Is this in dispute? No, not in dispute. What is a bit tricky is that map_get_seen can return TRUE for unknown tiles: this is a low-level function that should only be used in a few places. > > > P.S. Send future reports to the bug tracker. > > I *did* send the *report* to the tracker. Are you asking me to only send > future *patches* for open/new tickets to the tracker and not the list? > Do you want me to create a new ticket for each patch, like you > apparently did to respond to my earlier message? You sent the report to the tracker then sent the patch to the list. I erronously made a new report which I merged into the old once when I realized it already existed (hence the ugly subject line). Simply responding to the email from the bug tracker (with PR# in the subject) is generally sufficient for any reply. -jason ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#39330) Re: (PR#39335) Re: Fix for PR#39330
http://bugs.freeciv.org/Ticket/Display.html?id=39330 > Jason Short wrote: > http://bugs.freeciv.org/Ticket/Display.html?id=39335 > >>Attached to this message is a patch to fix the issue reported in >> PR#39330. Basically, the problem seems to be that cities can see unknown >> terrain, and this is considered acceptable and normal. But, in at least >> one place in the code that possibility is overlooked. In a player's view >> of a tile, the terrain ptr may be NULL (indicating it is unknown). >> Attempts to reference a member, such as 'type', of such a NULL ptr >> obviously results in a segfault. > > Good catch but the bug is deeper. We do NEED to know the terrain type > here since the tile is being sent as TILE_KNOWN_FOGGED. Can you find > a way to reproduce the problem and figure out why a "known" tile has > unknown terrain type? That should not be possible. Well, the patch can be easily be modified to send 'TILE_UNKNOWN' instead, for the case when the tile is seen by a city, but unknown. A *seen* tile is allowed to be *unknown* according to a comment in server/maphand.c, starting at line 810 on the SVN head revision, which I quote here: / Return whether the player can see the tile. Seeing a tile means you have vision of it now (as opposed to knowing a tile which means you've seen it before). Note that a tile can be seen but not known (currently this only happens when a city is founded with some unknown tiles in its radius); in this case the tile is unknown (but map_get_seen will still return TRUE). / Is this in dispute? > P.S. Send future reports to the bug tracker. I *did* send the *report* to the tracker. Are you asking me to only send future *patches* for open/new tickets to the tracker and not the list? Do you want me to create a new ticket for each patch, like you apparently did to respond to my earlier message? Eric ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
