richardwilkes / gcs

GURPS Character Sheet
http://gurpscharactersheet.com
Mozilla Public License 2.0
217 stars 60 forks source link

When a new skill is added determine what it defaults to first, before existing skills that might now have a default they could use #762

Closed XerWolf closed 1 month ago

XerWolf commented 10 months ago

The calculations around default skills are not automatic when adding a skill to a sheet and only seem to update when you increment the points associated to the skill. It's easy to reproduce this with 3 skills: performance, acting, public speaking.

richardwilkes commented 10 months ago

I am unable to reproduce any anomalies. Please provide exact steps you took, what you saw, and what you expected. Thanks.

XerWolf commented 10 months ago

Add performance to the sheet, set it to 20 points. Add Acting to the sheet. It shows as IQ-1. Expectation would be that Acting would default to performance - 2 until 4 points were spent in Acting and then it would be performance - 1 at that point. I'm seeing one directional defaults based on the skill that already exists on the sheet instead of the newly added skill. ~Would be nice to also be able to switch the direction of the default per page 173~

XerWolf commented 10 months ago

I see there is a right click option to swap the defaults now. I think the accomplishes most of this, but still strange it defaults to using the newly added skill as the default.

richardwilkes commented 10 months ago

I believe the reason you see this particular behavior is that the skills are swept through in order on the sheet and since Performance can now default to something, it does, then it moves on to the next. Since you can change the default, that seemed reasonable.

In the past, I put some effort into always selecting the most optimal defaults. However, I got a lot of push-back from folks that didn't want that behavior, hence the current method. There is also a lot of complexity when you have multiple skills that can all default to each other, which can make the automatic selections non-obvious and which can also constrain what is available to be swapped to.

Anyway, bottom line, I don't currently see anything I can do here that would actually improve things in the general case and still satisfy the majority of users. If you have a concrete suggestion that accounts for the many permutations of things that can happen, I'm open to it.

XerWolf commented 10 months ago

My only suggestion would be to have the newly added skill get its default before going through the list but if your processing steps would have to be rewritten to accomplish that then it's a minor enough thing that you can just close this.

XerWolf commented 10 months ago

I did notice when playing with the change default that if you change default on a skill that shifts back to the primary stat that it might lose the option to change default back, but found a work around for that.

richardwilkes commented 9 months ago

While certainly not impossible, the current code has no knowledge that something new was actually added when this task is performed. I'll keep this open for now in case I ever revise logic in this area, though.

Legendsmith commented 8 months ago

Personally I think the functionality, including the Swap Defaults option in the context menu is already fine as is, but this solution did occur to me. It'd say it could/should be a sheet setting.

Instead of just checking all skills, in descending order, check other skills within the same container first. then check the rest if none are found. Thus if someone wants some control over defaults, they can just stick skills they desire to default to each other, in descending order, within their own little container.

richardwilkes commented 1 month ago

Newly added skills now have their default chosen prior to existing skills being revisited for updated defaults. This fixes the case mentioned here... but still won't always give the best result if, say, multiple skills are dragged over at once (or applied from a template), since then all of them are "new" and those new skills will be visited in order -- so the same ordering problem still applies in that case. Other than reinstating the logic for forcing the "best" default, that can't be fixed, though... and I don't plan to reinstate that logic due to the previous push-back when it was there.