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 #921933
Production of obsolete buildings not really upgraded
0%
Description
The server sends messages like
Production of Barracks upgraded to Barracks II in City A City A is building Barracks(O) which is no longer available
It does get actually switched, city_build_building just uses the old value
when checking if it's still available.
History
#1
Updated by Anonymous 11 months ago
- File 0001-Fix-updating-production-of-obsolete-buildings-preven.patch 0001-Fix-updating-production-of-obsolete-buildings-preven.patch added
Fix by fetching the improvement being produced only after the upgrade.
Also prevent buying an obsolete building, it doesn't seem to make sense
anyway and could be used for a partial buy of the upgraded building.
#2
Updated by Marko Lindqvist 11 months ago
Ilkka Virta wrote:
Fix by fetching the improvement being produced only after the upgrade.
You should then leave it uninitialized until that point. As per CodingStyle:
"- Never initialize variables with values that make no sense as their
value in case they get used. If there's no sensible initialization
value for a variable, leave it uninitialized. This allows various
tools to detect if such a variable ever gets used without assigning
proper value to it."
Also prevent buying an obsolete building, it doesn't seem to make sense
anyway and could be used for a partial buy of the upgraded building.
That's a separate issue, which should be handled separately. It even adds a translatable string while the main issue does not -> it may cause patch as a whole to get postponed.
Recently Sveinung wrote something along the lines: "When you begin a sentence with 'Also', you usually should have a separate ticket"
#3
Updated by Anonymous 11 months ago
Marko Lindqvist wrote:
Ilkka Virta wrote:
You should then leave it uninitialized until that point.
Yep.
Also prevent buying an obsolete building, it doesn't seem to make sense
anyway and could be used for a partial buy of the upgraded building.That's a separate issue, which should be handled separately. It even adds a translatable string while the main issue does not -> it may cause patch as a whole to get postponed.
I did also add the 'city_can_change_build()' test, which already is a constraint
that didn't exist there before. (It also wouldn't have made a difference now if the
buy was prevented, but could matter if some other factor preventing changing
was added at some point.)
In any case, I do think it is rather an integral part of the update behaviour to
decide what happens if a player tries to buy the newly-obsolete building. Some of
the options to that are at odds with the general sense that it's not possible
to buy something partially, or to buy something and then change to something
else. Having the same behaviour as for units wouldn't be very useful either, since
getting the obsolete building would be totally useless and it would just get sold the
next time the player discovers a new tech. Hence, I would suggest just preventing
the buy.
But, I don't know what you think the proper behaviour should be, or if you consider
the current behaviour a bug to begin with, so I'm not sure if there's much more I
can do about this.
#4
Updated by Marko Lindqvist 11 months ago
Ilkka Virta wrote:
In any case, I do think it is rather an integral part of the update behaviour to
decide what happens if a player tries to buy the newly-obsolete building.
It's not integral part of fixing the erroneous message about the obsolete production. It's a separate issue.
The main fix should be able to get in fast. The piggybacking buying part prevents that.
#5
Updated by Marko Lindqvist 10 months ago
- Sprint/Milestone set to 2.6.5
S2_6:
git am ../../patch/0001-Fix-updating-production-of-obsolete-buildings-preven.patch
Applying: Fix updating production of obsolete buildings, prevent buying them
/fast/freeciv/master/src/.git/worktrees/src1/rebase-apply/patch:14: trailing whitespace.
if (VUT_IMPROVEMENT == pcity->production.kind
warning: 1 line adds whitespace errors.
Doesn't apply at all to master, have not tested branches in between.
#6
Updated by Marko Lindqvist 9 months ago
Can you look at updating this before we enter another freeze (for 3.0.0-beta2) ?
#7
Updated by Marko Lindqvist 7 months ago
- Sprint/Milestone changed from 2.6.5 to 2.6.6
#8
Updated by Marko Lindqvist about 2 months ago
Marko Lindqvist wrote:
Ilkka Virta wrote:
In any case, I do think it is rather an integral part of the update behaviour to
decide what happens if a player tries to buy the newly-obsolete building.It's not integral part of fixing the erroneous message about the obsolete production. It's a separate issue.
#9
Updated by Marko Lindqvist about 2 months ago
- File 0027-Fix-erroneous-message-about-obsolete-build-target-af.patch 0027-Fix-erroneous-message-about-obsolete-build-target-af.patch added
- File 0005-Fix-erroneous-message-about-obsolete-build-target-af.patch 0005-Fix-erroneous-message-about-obsolete-build-target-af.patch added
- Status changed from New to Resolved
- Minimal patch for all branches that should still be safe even for 2.6.6
#10
Updated by Marko Lindqvist about 2 months ago
- File 0038-Fix-erroneous-message-about-obsolete-build-target-af.patch 0038-Fix-erroneous-message-about-obsolete-build-target-af.patch added
- Master-only update to fix build
#11
Updated by Marko Lindqvist about 2 months ago
- Status changed from Resolved to Closed
- Assignee set to Marko Lindqvist
.