bugFreeciv - Bugs: bug #21159, 'mapimg colortest' causes...

 
 
Show feedback again

bug #21159: 'mapimg colortest' causes assertion failures and server disconnection

Submitted by:  Jacob Nevins <jtn>
Submitted on:  Mon Sep 23 09:41:02 2013  
 
Category: generalSeverity: 3 - Normal
Priority: 5 - NormalStatus: Fixed
Assigned to: Marko Lindqvist <cazfi>Open/Closed: Closed
Release: 2.4.0Operating System: GNU/Linux
Planned Release: 2.4.1,2.5.0,2.6.0Contains string changes: None

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Wed Oct 30 21:06:26 2013, SVN revision 23661:

Fixed mapimg colortest crash.

Reported by Dox4242

See bug #21159

(Browse SVN revision 23661)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Wed Oct 30 21:06:20 2013, SVN revision 23660:

Fixed mapimg colortest crash.

Reported by Dox4242

See bug #21159

(Browse SVN revision 23660)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Wed Oct 30 21:06:13 2013, SVN revision 23659:

Fixed mapimg colortest crash.

Reported by Dox4242

See bug #21159

(Browse SVN revision 23659)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Sun Oct 27 00:11:52 2013, comment #7:

Patches

(file #19254, file #19255)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Sat Oct 26 23:34:37 2013, comment #6:

I'm working on a fix that has essentially two parts:

- img_new() gets topology as parameter. topo_has_flag() takes topology argument too - new current_topo_has_flag() takes not (behaves like old topo_has_flag() )
- img_plot() takes coordinates instead of tile index (to fix mess in conversions between 3 different coordinate systems). colortest does not need dummy tile any more. New wrapper img_plot_tile() takes tile and converts to coordinates.

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Wed Sep 25 01:04:07 2013, comment #5:

> Fix as simple as adding a topology argument to img_new() rather
> than it using the global map.topology_id? (Haven't tried it.)


Global topology is accessed directly deeper in the helper functions, so I guess (I could be wrong: maybe the topology doesn't matter in those cases) it's not simply matter of img_new() knowing correct topology.

If there's no proper fix when 2.4.1 release date draws near, I would try ugly hack (tm) of "saved_topology = map.topology_id; map.topology_id = NONISO; ... map.topology_id = saved_topology;" as temporary solution.

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Tue Sep 24 23:41:45 2013, comment #4:

Fix as simple as adding a topology argument to img_new() rather than it using the global map.topology_id? (Haven't tried it.)

Jacob Nevins <jtn>
Project Administrator
Tue Sep 24 07:44:48 2013, comment #3:

Aha, that would neatly explain why I didn't spot it when polishing mapimg/playercolor -- the default was changed to iso quite late on.

Jacob Nevins <jtn>
Project Administrator
Tue Sep 24 01:13:22 2013, comment #2:

It suspiciously uses common tile coordinate / index conversion functions etc while not targeting to the same shape and dimensions as current map has - and indeed, by setting map topology to non-iso before running 'mapimg colortest', it success without assertions.

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Mon Sep 23 09:41:50 2013, comment #1:

(I have MagickWand compiled in, no idea about the original reporter.)

Jacob Nevins <jtn>
Project Administrator
Mon Sep 23 09:41:02 2013, original submission:

Reported by Dox4242 on the forum, reproduced by me:

In a client-spawned server, '/mapimg colortest' in a running game leads to the client/server connection being broken with the client saving 'Lost connection to server (decoding error)!'

In a standalone server, doing '/mapimg colortest' pregame, there are loads of assertion failures including repeats of the following, but the server stays alive:

Jacob Nevins <jtn>
Project Administrator

 

(Note: upload size limit is set to 1024 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #19254:  ColortestCrash.patch.bz2 added by cazfi (6kB - application/x-bzip)
file #19255:  ColortestCrash-S2_5.patch.bz2 added by cazfi (6kB - application/x-bzip)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by cazfi (Posted a comment)
  • -unavailable- added by jtn (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Error: not logged in

     

     

    Follow 8 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed Oct 30 21:06:39 2013cazfiStatusReady For Test=>Fixed
      Open/ClosedOpen=>Closed
    Sun Oct 27 00:11:52 2013cazfiAttached File-=>Added ColortestCrash.patch.bz2, #19254
      Attached File-=>Added ColortestCrash-S2_5.patch.bz2, #19255
      StatusIn Progress=>Ready For Test
    Sat Oct 26 23:34:37 2013cazfiCategoryNone=>general
      StatusConfirmed=>In Progress
      Assigned toNone=>cazfi
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup