Project

Profile

Help

HostedRedmine.com has moved to the Planio platform. All logins and passwords remained the same. All users will be able to login and use Redmine just as before. Read more...

Bug #870966

get_building_bonus() [effects.c::807]: assertion '((void *)0) != pcity && ((void *)0) != building' failed.

Added by Zoltán Žarkov over 2 years ago. Updated over 2 years ago.

Status:
Closed
Priority:
Normal
Category:
Client
Sprint/Milestone:
Start date:
Due date:
% Done:

0%

Estimated time:

Description

There are a few other issues filed for this one, but maybe none addressed the underlying problem.

get_city_dialog_production_row is supposed to tolerate pcity NULL, but impr_build_shield_cost passes along NULL pcity to get_building_bonus, which asserts pcity is not NULL.

qt backtrace:
3: 5: ./client/freeciv-qt(fc_assert_fail+0x2a6) [0x562ed1c231b2]
3: 6: ./client/freeciv-qt(get_building_bonus+0x95) [0x562ed1967894]
3: 7: ./client/freeciv-qt(impr_build_shield_cost+0x68) [0x562ed1980601]
3: 8: ./client/freeciv-qt(get_city_dialog_production_row+0x563) [0x562ed185a2d6]
3: 9: ./client/freeciv-qt(_ZN11city_widget21gen_production_labelsENS_11menu_labelsER4QMapI7QStringiEbbPFbPK4cityPK9universalEb+0x92f) [0x562ed16686f9]

gtk backtrace:
3: 5: ./client/freeciv-gtk3.22(fc_assert_fail+0x2a6) [0x563a95997009]
3: 6: ./client/freeciv-gtk3.22(get_building_bonus+0x95) [0x563a956db6c1]
3: 7: ./client/freeciv-gtk3.22(impr_build_shield_cost+0x68) [0x563a956f442e]
3: 8: ./client/freeciv-gtk3.22(get_city_dialog_production_row+0x563) [0x563a955d74d8]
3: 9: ./client/freeciv-gtk3.22(+0x3cd7c8) [0x563a955be7c8]


Related issues

Related to Freeciv - Feature #767451: "Building_Build_Cost_Pct" effectClosed

<a title="Actions" class="icon-only icon-actions js-contextmenu" href="#">Actions</a>
Related to Freeciv - Feature #871844: impr_estimate_build_shield_cost()Closed

<a title="Actions" class="icon-only icon-actions js-contextmenu" href="#">Actions</a>

History

#1 Updated by Zoltán Žarkov over 2 years ago

#2 Updated by Zoltán Žarkov over 2 years ago

Regression originates from #767451

#3 Updated by Marko Lindqvist over 2 years ago

  • Sprint/Milestone set to 3.1.0

It's a master-only change that impr_build_shield_cost() requires valid city.

#5 Updated by Marko Lindqvist over 2 years ago

Untested patch attached.

This patch adds generic impr_base_build_shield_cost() function and uses it. Some earlier fixed cases of the same assert did the functionality in the caller side - those too could be changed to use this new function (in a new ticket)

#6 Updated by Zoltán Žarkov over 2 years ago

If you are doing it this way, make sure to change all the other places where null pcity is possibly passed, like in helpdlg code. I still think that when showing players potential build costs it is better to give a cost estimate that accounts for player-wide effects even when city is unspecified.

#7 Updated by Marko Lindqvist over 2 years ago

Zoltán Žarkov wrote:

If you are doing it this way, make sure to change all the other places where null pcity is possibly passed, like in helpdlg code. I still think that when showing players potential build costs it is better to give a cost estimate that accounts for player-wide effects even when city is unspecified.

You have a point. My point for doing this in caller side is that callers should be aware when they are not necessarily getting the real cost (as they used to) but an base cost only.
Maybe we can have best of the both worlds? (Probably asks for splitting this ticket to two separate ones)

#8 Updated by Marko Lindqvist over 2 years ago

Marko Lindqvist wrote:

Maybe we can have best of the both worlds? (Probably asks for splitting this ticket to two separate ones)

-> Feature #871844

#9 Updated by Marko Lindqvist over 2 years ago

#10 Updated by Marko Lindqvist over 2 years ago

Zoltán Žarkov wrote:

where null pcity is possibly passed, like in helpdlg code.

-> Bug #872018

#11 Updated by Marko Lindqvist over 2 years ago

  • Status changed from Resolved to Closed
  • Assignee set to Marko Lindqvist

Also available in: Atom PDF