bugFreeciv - Bugs: bug #20706, Rates dialog tidy-ups: limit to...

 
 
Show feedback again

bug #20706: Rates dialog tidy-ups: limit to 100%, not setting increment for rates_lux_scale

Submitted by:  Marko Lindqvist <cazfi>
Submitted on:  Wed 03 Apr 2013 08:50:11 PM UTC  
 
Category: client-gtk-3.0Severity: 3 - Normal
Priority: 5 - NormalStatus: Fixed
Assigned to: Jacob Nevins <jtn>Open/Closed: Closed
Release: Operating System: Any
Planned Release: 2.4.0-beta2, 2.5.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)

Fri 26 Apr 2013 12:05:44 AM UTC, SVN revision 22762:

In Gtk3 rates dialog, change range from 110% to 100%, and set increments
correctly for luxury slider.

Based on a patch by Rafa?\197?\130 Mu?\197?\188y?\197?\130o (galtgendo@gna) and a report by
Marko Lindqvist (cazfi@gna).

See gna bug #20706.

(Browse SVN revision 22762)

Jacob Nevins <jtn>
Project AdministratorIn charge of this item.
Fri 26 Apr 2013 12:02:47 AM UTC, SVN revision 22761:

In Gtk3 rates dialog, change range from 110% to 100%, and set increments
correctly for luxury slider.

Based on a patch by Rafa?\197?\130 Mu?\197?\188y?\197?\130o (galtgendo@gna) and a report by
Marko Lindqvist (cazfi@gna).

See gna bug #20706.

(Browse SVN revision 22761)

Jacob Nevins <jtn>
Project AdministratorIn charge of this item.
Thu 25 Apr 2013 11:52:41 PM UTC, comment #10:

> Can you check the handling of EFT_MAX_RATES less than 40 (30% +
> 30% + 30% would not make up to 100%)

Bit hopeless.
Server says "in player_limit_to_max_rates() [plrhand.c::1656]: assertion 'surplus % 10 == 0' failed."
In Gtk3 client, rates dialog starts out at all zeroes but limited to 30.
In Gtk2 client, the same except without the limit.
It's not clear what the right answer would be, but there's at least one new ticket in this.

> or higher than 100?

Both Gtk2 and Gtk3 seemed to cope fine with this (handling it the same as 100% as far as I could tell).

Jacob Nevins <jtn>
Project AdministratorIn charge of this item.
Tue 16 Apr 2013 09:34:38 PM UTC, comment #9:

Can you check the handling of EFT_MAX_RATES less than 40 (30% + 30% + 30% would not make up to 100%) or higher than 100? That may lead to another ticket if you so decide. Implementation is also likely to be identical between gtk2- and gtk3-clients, so if one is buggy, the other likely is too.

Marko Lindqvist <cazfi>
Project Administrator
Tue 16 Apr 2013 08:23:40 PM UTC, comment #8:

So well um. Anyone going to object if I commit this patch as-is?

(I tried briefly to find code where 110% was treated specially, and failed.)

Jacob Nevins <jtn>
Project AdministratorIn charge of this item.
Thu 11 Apr 2013 09:39:50 PM UTC, comment #7:

btw Noticed that freeciv-web has interface where entire tax rates dialog is constructed so that max value for the ruler is current maxrate value.

Marko Lindqvist <cazfi>
Project Administrator
Mon 08 Apr 2013 07:38:19 PM UTC, comment #6:

> open rates dialog and rates setting icons in left panel are no way linked


They never were AFAIK.
Rates dialog checks the values only upon its creation.
Trying to synchronize them would make the code much more complicated for very little gain.

Similarly, if i.e. the dialog was open and upon end of turn government changed, some of the data would have been displayed incorrectly till the dialog was closed and reopened.

Any communication between those two would be tricky, as rates dialog doesn't exist all of the time, like the icons do.

Rafał Mużyło <galtgendo>
Mon 08 Apr 2013 06:05:44 PM UTC, comment #5:

I gave Democracy Max_Rates effect of 110 and tested the dialog with that. Value displayed remained as 100% when I moved between 100 and 110 (and neither of the two other rates went negative). I noticed no effects, and I think the value gets cropped to 100% anyway.

While playing around with the dialog: open rates dialog and rates setting icons in left panel are no way linked - change in one does not cause the other to refresh.

Marko Lindqvist <cazfi>
Project Administrator
Mon 08 Apr 2013 12:08:10 PM UTC, comment #4:

Well, it would be much easier if I could remember where I've seen that special condition (IIRC, 110% was not a real value, but an indicator of a special status (civil war ?)), nevertheless 110% was already valid before my patch - the marks just made it more obvious.

Rafał Mużyło <galtgendo>
Mon 08 Apr 2013 11:39:08 AM UTC, comment #3:

I don't think player should be ever allowed to set rates higher than 100%.

I assume that you're referring to the fact that max rates effect can have any value coming from the ruleset. The fact that this code appears to have a bug of not cropping max rate to 0-100 but will use any effect value (not just 0-110) direcly also hints to that direction.
For example in alien ruleset total of 110% effect value is possible when multiple effects stack.

Marko Lindqvist <cazfi>
Project Administrator
Mon 08 Apr 2013 11:14:34 AM UTC, comment #2:

Ah, no.
110% is deliberate - not by my design, it's freeciv that reports 110% to indicate a special condition. I remember stumbling upon this back when I was writing those patches, but I no longer recall what condition exactly was it.

Rafał Mużyło <galtgendo>
Sat 06 Apr 2013 09:39:05 PM UTC, comment #1:

(I think the main effect of the existing change is to allow the arrow keys to work for luxury?)

> Noticed that there's 12 steps marked. 0%, 10% ... 110% ?
> Tested only with Despotism as government, so was unable to get
> past 60% mark.

(My standard dodge for quickly testing governments is to build a city with an initial settler then use the editor to put Statue of Liberty there.)

Indeed, testing with Democracy shows that we have an extra superfluous checkmark. Making the obvious changes to remove it doesn't have any obvious ill effect.

Attached a patch which incorporates the previous one and adds this change (which would make this into a general "fix Gtk3 rates dialog" ticket, but it doesn't seem worth separating).

(Another very minor thing I notice is that with the Freeciv theme, clicking around the rates dialog, the papery background seems to "crawl" a bit.)

(file #17689)

Jacob Nevins <jtn>
Project AdministratorIn charge of this item.
Wed 03 Apr 2013 08:50:11 PM UTC, original submission:

In patch #3469 Rafał Mużyło <galtgendo> mentions typo:
"In client/gui-gtk-3.0/gamedlgs.c, around line 278, in rates_lux_scale block, in gtk_range_set_increments call, rates_sci_scale is used by mistake."

Fix attached

Noticed that there's 12 steps marked. 0%, 10% ... 110% ? Tested only with Despotism as government, so was unable to get past 60% mark.

Marko Lindqvist <cazfi>
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 #17689:  trunk-S2_4-gtk3-ratesdlg-scales.patch added by jtn (3kB - text/x-diff - trunk/S2_4 r22677: set range to 100% additionally)
file #17672:  LuxScale.patch added by cazfi (645B - text/x-diff)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by jtn (Updated the item)
  • -unavailable- added by cazfi (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
    Fri 26 Apr 2013 12:07:39 AM UTCjtnAssigned toNone=>jtn
    Fri 26 Apr 2013 12:07:29 AM UTCjtnStatusReady For Test=>Fixed
      Open/ClosedOpen=>Closed
      SummaryNot setting increment for rates_lux_scale=>Rates dialog tidy-ups: limit to 100%, not setting increment for rates_lux_scale
    Sat 06 Apr 2013 09:39:05 PM UTCjtnAttached File-=>Added trunk-S2_4-gtk3-ratesdlg-scales.patch, #17689
      Operating SystemNone=>Any
      Planned Release2.4.0, 2.5.0=>2.4.0-beta2, 2.5.0
    Wed 03 Apr 2013 08:50:11 PM UTCcazfiAttached File-=>Added LuxScale.patch, #17672
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup