Opened 10 years ago

Closed 8 years ago

#938 closed bug (fixed (in master))

Non-maximise mode does not work

Reported by: Phil, r.g.r.a Owned by:
Milestone: 3.2.0 Keywords: birth
Cc:

Description

If I switch off the birth option to maximise race/class bonus, when I create my character, it seems to ignore the race class bonus.
The bonuses show up ok when rolling or doing point-buy, but once in town the stats are very different indeed.

Attachments (1)

md_938.patch (29.2 KB) - added by Marble Dice 9 years ago.
Adds new birth CMD to finalize birth options immediately after roller selection

Download all attachments as: .zip

Change History (16)

comment:1 Changed 10 years ago by takkaria

  • Milestone changed from Triage to 3.1.2 beta
  • Reporter changed from anonymous to Phil, r.g.r.a

comment:2 Changed 10 years ago by magnate

  • Keywords birth added

comment:3 follow-up: Changed 9 years ago by magnate

Isn't maximise something to mainstream, like autoscum?

Changed 9 years ago by Marble Dice

Adds new birth CMD to finalize birth options immediately after roller selection

comment:4 in reply to: ↑ 3 Changed 9 years ago by Marble Dice

Replying to magnate:

Isn't maximise something to mainstream, like autoscum?

Mainsteam as in make standard and get rid of the option? I'd never turn it off, but as long as you can disable maximise, it might as well work as advertised.

This problem happens because the adult_maximize option isn't finalized from the birth_maximize option until after the character is accepted. The birth process and the game at large check the adult settings rather than the birth setting.

A solution would be use the birth_maximize option during the birth process, or finalize the adult_maximize option after roller selection. Since parts of the birth ui seem to invoke parts of the regular character screen ui (which always needs to check the adult option) to display stats, it would probably make more sense to finalize the adult options before the player can muck around with buying or rolling for stats.

comment:5 Changed 9 years ago by magnate

  • Milestone changed from 3.1.2 beta to 3.1.3
  • Status changed from new to confirmed

Better to follow #599, apparently.

comment:6 Changed 8 years ago by magnate

  • Status changed from confirmed to pending

I have applied MarbleDice?'s fix in https://github.com/magnate/angband/commit/629b4f4e56cf9f3bb51b9c0f0e9641b466fcc781. Setting to pending until that is cherry-picked into staging/master

comment:7 Changed 8 years ago by magnate

Ok I decided to push this into staging so takkaria can see it along with the other pending fixes: http://bit.ly/h2dbpi.

comment:8 Changed 8 years ago by magnate

  • Status changed from pending to confirmed
  • Summary changed from problem with character creation and birth options to Non-maximise mode does not work

Nope, this is not fully fixed yet. The fix above works for new characters created from scratch, but not for characters based on the previous character. For some reason the race and class boni are not applied in that case. Needs further investigation

comment:9 Changed 8 years ago by magnate

  • Summary changed from Non-maximise mode does not work to Quickstart does not work properly in non-maximise mode

Ok. The actual operation of non-maximise mode is fine: the quickstarted character's stats correctly include the RB and CB. The problem is that the stats are not what the previous character started with, they are always 17/10/10/17/17/10, for any race and class.

comment:10 Changed 8 years ago by magnate

  • Summary changed from Quickstart does not work properly in non-maximise mode to Quickstart does not work properly

At last, light dawns. The problem is not specific to non-maximise: the fix above breaks quickstart altogether (the points are not spent). Still working on it ...

comment:11 Changed 8 years ago by magnate

No, it's not even that simple - quickstart seems to work ok in maximise mode unless you just changed mode. This is driving my crazy, so I'm going to document it here for my peace of mind.

We start in dungeon.c with the call to player_birth().

In player_birth(), we initialise some null variables and then check if we're doing a quickstart. If we are, we call save_roller_data(), which saves the BIRTH stats of the previous character (p_ptr->stat_birth[i]). This should cater for both maximise and non-maximise correctly: the birth stats include RB and CB in the latter case and not in the former. So we have the birth stats saved in a "birther" structure if we had a previous character.

The we call reset_stats(), which sets all stats to 10, points spent to 0 and points left to max (24). It then calls recalculate_stats() which has the first check for OPT(adult_maximize): if it's true it sets all of stat_cur, stat_max and stat_birth to 10. If we're in non-maximize it adjusts the 10 by RB and CB (I assume that we use the race and class of the previous dead character at this point, or they'd be zero for a brand new savefile) - but it only stores the result in stat_cur and stat_max, not stat_birth. It then footles with gold and updates and returns us to reset_stats() which in turn signals another event and returns us to player_birth().

We then call do_birth_reset() which calls player_init() (which zeroes a whole bunch of stuff like known flavours, kills etc. but doesn't touch stats) and then calls load_roller_data() if quickstart is allowed. This sets all of stat_max, stat_cur and stat_birth to the stored birth stats from above, and returns us to do_birth_reset() which then calls player_generate() (which again sets some stuff but doesn't affect stats), updates bonuses and returns us to player_birth().

So at this point there are four scenarios:

  1. Quickstart is false, maximise is true: p_ptr->stat_cur, stat_max, stat_birth are all 10, for all stats.
  1. Quickstart is false, maximise is false: p_ptr->stat_cur and stat_max are 10+RB+CB, stat_birth is unset.
  1. Quickstart is true, maximise is true: all of cur/max/birth are set to the previous player's birth stats.
  1. Quickstart is true, maximise is false: same as 3, since load_roller_data() is called after the check for maximise in recalculate_stats(), so any differences are overwritten by the previous birth stats.

Then we try to increment the name suffix and enter the get_birth_command() loop in ui-birth.c. The first stage is BIRTH_RESET, which means that reset_stats() and do_birth_reset() are called again. I have tested removing the two calls above and everything still works, so I am pretty certain they are redundant.

Then we test for quickstart and either go to it, or skip ahead to choosing sex. The quickstart step starts with calling display_player(0), and this is where the bug becomes apparent: the stats are displayed, and are clearly not right. They ARE correct in maximise mode, but not in non-maximise. Therefore the problem is the storing of "birth" stats: they are not stored correctly in non-maximise mode. Ok, good. Now I'm off to find out where they're set.

comment:12 Changed 8 years ago by magnate

Hah. They are of course set in recalculate_stats(), so that was the bug. Adding stat_birth to the non-maximise branch solves this problem: the correct stats are remembered in non-maximise mode.

But there is a separate bug, which is to do with the maximise option being toggled during the life of the previous character, rather than during the birth process. This needs another call to CMD_FINALIZE_OPTIONS so that a quickstart picks up a change to the option.

comment:13 Changed 8 years ago by magnate

Hmmm. But that will actually mess everything up, as the stored birth stats will be wrong if the option is toggled. I think it's best to leave quickstart as ignoring any changes to birth options. This will confuse people, but the whole concept of "birth" options confuses people - IMO the best solution is to make them accessible during the non-quickstart birth process and make them INaccessible at any other time. Then there can be no confusion between birth_* options and adult_* options.

comment:14 Changed 8 years ago by magnate

  • Status changed from confirmed to pending
  • Summary changed from Quickstart does not work properly to Non-maximise mode does not work

Ok, finally sorted in http://bit.ly/fJgE4Y. If it is desired that quickstart mode picks up changes to birth options, some ugly hackery is required.

comment:15 Changed 8 years ago by magnate

  • Resolution set to fixed
  • Status changed from pending to closed
Note: See TracTickets for help on using tickets.