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 23 Sep 2013 09:41:02 AM UTC  
 
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.0

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 30 Oct 2013 09:06:26 PM UTC, 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 30 Oct 2013 09:06:20 PM UTC, 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 30 Oct 2013 09:06:13 PM UTC, 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 27 Oct 2013 12:11:52 AM UTC, comment #7:

Patches

(file #19254, file #19255)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Sat 26 Oct 2013 11:34:37 PM UTC, 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 25 Sep 2013 01:04:07 AM UTC, 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 24 Sep 2013 11:41:45 PM UTC, 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 24 Sep 2013 07:44:48 AM UTC, 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 24 Sep 2013 01:13:22 AM UTC, 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 23 Sep 2013 09:41:50 AM UTC, comment #1:

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

Jacob Nevins <jtn>
Project Administrator
Mon 23 Sep 2013 09:41:02 AM UTC, 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.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 8 latest changes.

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

    Back to the top


    Powered by Savane 3.1-cleanup