Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tweaking skill rust to fix learning overall #50176

Merged
merged 45 commits into from
Aug 12, 2021

Conversation

I-am-Erk
Copy link
Member

@I-am-Erk I-am-Erk commented Jul 23, 2021

Summary

features "Transforms skill rust from a nuisance to a robust learning simulator."

Purpose of change

So, I had an epiphany today and then Kevin double-epiphanied it.

We've been trying for a long time to improve the way we model learning, to model two types of learning: let's call it "knowledge" and "skill" (practical skill, specifically). I've also wanted to simulate the fact that people learn better if they take breaks in learning and come back to something.

Describe the solution

The epiphany was this: Skill rust already does this, almost entirely. You lose levels of a skill, but gain them back faster than it took the first time.

We only need a few changes.
1: when you recover experience you had lost due to rust, you also increase the total experience you had to begin with. So, while you wind up in a worse spot, you come out ahead in the end. Practising a skill that has rusted is beneficial.
2: Books do not increase your current experience. Instead, they increase your theoretical maximum experience.

Here is the nitty gritty of that:

  • Track the highest amount of xp you have ever obtained in a skill
  • if you are currently below that, gain xp at an increased rate and also increase the tracker for highest amount of xp ever gained by a bonus amount

Basically, we treat your current "rusted" skill+xp level as "practical skill". The highest level+xp you have reached ever is your "theoretical knowledge". Knowledge level should affect unlocked recipes and whether or not you can even attempt to craft something, but practical skill determines your actual chance of success.

What this results in is a system where you are encouraged to learn the theory behind something, then go practice it, and when you do so you will gain XP noticeably faster. Likewise, you will learn faster if you take a break instead of sticking your nose to the grind. However, it doesn't penalize you particularly to keep pushing at the cutting edge if you have to.

As an aside, I am doing two additional things:

  • your rate of skill rust will decrease the further you are from your previous highest experience. At high XP levels this will mean it's hard to rust all the way down to another level.
  • I took out an unnecessary use of roll_remainder because we want to kill that process anyway.

Updated final result after feature creep due to excitement has settled down

Here is the new system for rust as I have it so far (28 July 2021).

New Rust

Rust is updated every 24 hours. I have removed all things that added a random ticker to rust to slow the rate, it happens every 24 hours regardless of if you have practised or not. At some point, we may want to change this to 24 hours from last time you practised, but that code is pretty alien to me.

When rust ticks, you lose an amount of xp equal to 4% of your current practical level, to start. That is added to a stat, rust_accumulator, which tallies up how much rust you've built up in total (and goes down when you practice, removing rust). Your xp loss each day is divided by sqrt(rust_accumulator/level_exp) clamped such that it is at most 4%, but once that denominator starts to increase, your rust rate begins to fall off. There is a hard clamp on 3 level's worth of XP due to rust.

The main bit I still need to run numbers on is how this will respond as your rust takes you down levels. At low levels this should still allow you to lose >1 level, but at higher levels I believe it will clamp off after a single level. That's because when level_exp is the xp required for your current level, and that's _level^2+10000, which means the gap in xp drops a ton as your levels drop off, dramatically slowing rust rate and potentially hitting that clamp value of level_exp*3 very quickly. I believe this is a desired behaviour, but I would like to see how it plays out.

Currently, I suspect we will find that this leads to too slow of rust in actual play. We may want the ticker to update to every 12 hours or so, and make it so it doesn't run rust if you practised the skill at all in those 12 hours.

Knowledge and skill

Whenever you practice a skill, you gain both knowledge and practical experience. When the two are equal, the game behaves the same as it always did. When your knowledge is ahead of your practical, you get a bonus to experience on both: your practical increases much faster (baseline double the normal rate, higher if you have higher int/per and always more than 1x even if you're real dumb). Your knowledge exp increases faster as well, meaning taking your rust away boosts your "high" skill level more than if you didn't have the rust in the first place. This is based currently on your int, and in the future I'd like some other mutations to impact it. Like the practical multiplier, it's always >1. The speed by which practical gains on knowledge increases as the gap between them widens, and falls off when the two are the same level as the XP difference between them closes.

Books currently only train knowledge, and that's the big nerf here. In order to prevent some weird interactions, I have tried to implement it such that your knowledge level determines things like what you can attempt with recipes and constructions, but not your rate of success. That means you can read up on mechanics until you know how to do something, but your chance of success will still require that you try it, maybe a few times. It also means that trying it out will give you a lot more xp than it used to.

As your knowledge level exceeds your practical when studying from a book, the rate of knowledge gain slows. It won't hit really penalizing levels unless your book larnin' gets astronomically high, but this should discourage sitting inside and studying 5 levels of tailoring before picking up a needle and thread.

Presently, NPC teachers also only teach knowledge. I would like that to include a bit of practical XP as well but I think that will be better put off until practice actions are a thing, because ideally I'd like NPC teachers to guide you in some practice actions, with the appropriate resource costs and things. They can't teach you practical skills if they don't have the tools around, any more than a book can.

I'm sure I have missed a few implementations of the new knowledge-practical stuff, and there will be some weird interactions that need shaking out. Additionally, we probably want to look into times activities take, and have you slow down if you have a knowledge level that dramatically exceeds your practical one.

To-Do

  • Remove unnecessary roll_remainder
  • Make skill rust repair increase your total max skill level after rust
  • Make reading books increase your knowledge level, not your practical level
  • Fix GUI report when reading books to show increase in knowledge
  • Fix GUI to show knowledge/practical levels more clearly (may be beyond my ability)
  • Make your rust rate decline the further you are from your knowledge maximum
  • Make rust not kick in instantly after game start not really a big problem now that it takes 24 hours, and should be improved later by making professions grant xp rather than skill levels.
  • Stop rust when you hit practical level 10
  • Turn rust on by default Probably I'll wait on that one
  • Change message for rust to sound less bad

Describe alternatives you've considered

Not being awesome?

Testing

  • Make sure your knowledge level unlocks actions like doing recipes (UI could use work)
  • Test learning by books - increase knowledge but not skill. Should apply to followers too.
  • Lower practical level slows knowledge gain from books
  • Knowledge level determines what books you can read
  • UI is pretty much legible
  • Can't learn from a book if knowledge is maxed even if practical is not
  • Skill rusts 4% per day or so, dropping off as level gap increases

Additional context

I could probably turn skill rust on by default now, it's not too bad.

It has been noted that this will make for some annoying situations where you can't gain practical skill levels until you advance to a higher knowledge level (eg. mechanics 6). That needs to be solved with the addition of practice actions, which should be done ASAP once this is ready. I am happy to turn that into a release blocker.

The @ menu looks pretty chunky right now, but it works I think. Your knowledge level display doesn't show unless there's a level gap, or your knowledge xp is at least 25% higher than practical.
image

Future directions

We need a lot more info in the @ menu now. We should consider making it tabbed, and have a new tab for "skills and proficiencies". Your main tab would only show your practical level, and we'd move proficiencies entirely to the new window, where we could also display them with requirements and more information.

@I-am-Erk I-am-Erk added [C++] Changes (can be) made in C++. Previously named `Code` Mechanics: Effects / Skills / Stats Effects / Skills / Stats labels Jul 23, 2021
@actual-nh
Copy link
Contributor

actual-nh commented Jul 24, 2021

Will the "disable skill learning in one skill to focus on another" silliness (from my viewpoint as an educator) be removed? EDIT: Why I'm asking on this in this PR is that currently disabling skill learning disables skill rust as well (OOPS - not correct). EDIT2: This is more a matter of focus drain from learning; never mind for now.

@I-am-Erk
Copy link
Member Author

Will the "disable skill learning in one skill to focus on another" silliness (from my viewpoint as an educator) be removed? EDIT: Why I'm asking on this in this PR is that currently disabling skill learning disables skill rust as well.

It won't be removed in this post but I have an issue for how to redo focus that will remove it. Basically it's a terrible feature that's a stopgap for focus being pretty chunky, once focus is better we can eliminate it.

src/skill.cpp Outdated Show resolved Hide resolved
src/character.cpp Outdated Show resolved Hide resolved
@I-am-Erk
Copy link
Member Author

I-am-Erk commented Jul 28, 2021

There are still some bugs to iron out with book reading and things, but this is very close to done if anyone wants to play around with it for now.

I did manage to get the effects of attributes in. I will write up a description of how this all works soon.

Copy link
Contributor

@actual-nh actual-nh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few things.

src/character.cpp Outdated Show resolved Hide resolved
src/character.cpp Outdated Show resolved Hide resolved
src/skill.cpp Outdated Show resolved Hide resolved
@I-am-Erk I-am-Erk marked this pull request as ready for review July 29, 2021 01:53
@actual-nh actual-nh added <Enhancement / Feature> New features, or enhancements on existing PUBLIC TEST Come and try this! labels Jul 29, 2021
@I-am-Erk
Copy link
Member Author

This is pretty much ready but I would dearly love some play testing. I am not going to have access to a compiler for a few weeks, and the scope is a lot for me to test the whole thing myself.

Also i could use some help with fixing tests.

@actual-nh actual-nh mentioned this pull request Jul 30, 2021
11 tasks
@@ -2369,6 +2369,13 @@ void Character::practice( const skill_id &id, int amount, int cap, bool suppress
return;
}

// Your ability to "catch up" skill experience to knowledge is mostly a function of intelligence,
// but perception also plays a role, representing both memory/attentiveness and catching on to how
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would perception really be involved here? Wouldn't something like natural physical prowess (dexterity = muscle memory?) tie to more physical/reflex based skills? Intelligence to skills requiring more complex understanding of concepts and ideas? I'm a bit unclear as to how perception factors in here... attentiveness is more of a factor of focus and mental fortitude (intelligence?) isn't it?

Now if we were talking about learning something through observation of someone demonstrating something to you, then both perception (ability to notice fine details) and intelligence (ability to understand those details) would make sense.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Focus, yes. Attentiveness? Plenty of people are intelligent but have problems paying attention - ADHD/ADD. Differentiating between skills in how they're affected by this split is planned.

@eltank
Copy link
Contributor

eltank commented Jul 30, 2021

it happens every 24 hours regardless of if you have practised or not

Just a thought: how about applying skill rust during sleep? That's something that tends to happen roughly every 24 hours and is a time when the brain "shuts down". The amount of rust would depend on the time since the last rust, which can be saved somewhere. Maybe with a forced rust application if a character avoids sleep for say 48 hours.

@eltank
Copy link
Contributor

eltank commented Jul 30, 2021

Suggestions for slimming down the UI (less is more):

computers: 5/6 (93%)
or
computers: 6->5 (93%)
or
computers: 5[6] (93%)

Basically try to keep it on one line. And when the player selects that line you can display additional information such as the theoretical % in the info box.

@I-am-Erk
Copy link
Member Author

I-am-Erk commented Jul 30, 2021

it happens every 24 hours regardless of if you have practised or not

Just a thought: how about applying skill rust during sleep? That's something that tends to happen roughly every 24 hours and is a time when the brain "shuts down". The amount of rust would depend on the time since the last rust, which can be saved somewhere. Maybe with a forced rust application if a character avoids sleep for say 48 hours.

It's not good to use sleep as a trigger because not all characters will sleep.

Basically try to keep it on one line. And when the player selects that line you can display additional information such as the theoretical % in the info box.

The current skill dialogue doesn't really have any characters to spare for even a more concise same-line display, and I definitely don't have the knowledge to make a view that expands when you select it. The UI right now is just "what can I do to display this info for now until someone who gets this code can do something better"

@kevingranade
Copy link
Member

I have a local check out of this that's rebased on current master and I'm investigating the test failures.
There was one test that tried to set skill higher than knowledge, which doesn't work because it bumps up knowledge to equal skill.
The code that makes knowledge keep pace with skill in skill::train() is making knowledge advance faster than intended, which is skewing some tests.
There's another effect making skill increases faster than intended that I don't have triaged yet.

@kevingranade kevingranade force-pushed the learning branch 2 times, most recently from 9e1ded2 to 2ce8965 Compare August 9, 2021 00:40
@kevingranade
Copy link
Member

It turned out to be three small adjustments away from at least being consistent with the existing tests.
One test was making an assertion about behavior when practical skill is higher than knowledge, which isn't a valid scenario.
The "time to gain skill levels" tests were all failing because while grinding on practice, the "catchup" code was kicking in inappropriately.
One trigger of this occurred when practical skill level passed knowledge skill without resetting knowledge skill practice, then the remaining knowledge experience would trigger catchup of practical practice on subsequent practice actions.
The other was a miscalculation of the catchup threshold that seemed to be based on treating knowledge xp as a non-resetting accumulator instead of a value that is reset when a new level is reached.

@kevingranade
Copy link
Member

The rust code seems to have a divide by zero error, looking into it.

delta % ( skill_rate * char_rate ) != 0_turns ) {

if( _level >= MAX_SKILL ) {
// don't rust any more once you hit the level cap, at least until we have a way to "pause" rust for a while.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a fairly straightforward solution is to allow exercise to progess up to X% of the way to MAX_LEVEL + 1, but not display this progress or allow MAX_LEVEL + 1 to be reached, then there's a whole level worth of exercise you can gain to buffer losing that last level.

@kevingranade
Copy link
Member

Unless I'm confused (possible!), the div0 was kind of a symptom of another error, which was calculating the "number of xp in the current skill level" based on the current level instead of level + 1 as it's done elsewhere (in Skill::train()). This leads to a minimum level value of 1 instead of zero and no div0.

Side note and totally ok for another PR, we really need an "alternating reading and practicing" test and some skill rust tests.

src/skill.cpp Outdated Show resolved Hide resolved
src/skill.cpp Outdated Show resolved Hide resolved
@lgtm-com
Copy link

lgtm-com bot commented Aug 10, 2021

This pull request introduces 1 alert when merging ccd4f1f into ffd812e - view on LGTM.com

new alerts:

  • 1 for Use of c-style math functions

src/skill.cpp Outdated Show resolved Hide resolved
phase 1: Track highest xp ever reached, recover rust faster if you are below it, and also gain xp while you regain rust.
increment highestlevel if your highestexperience gets there.
@I-am-Erk
Copy link
Member Author

Thanks for the finishing touches Kevin!

@I-am-Erk I-am-Erk deleted the learning branch August 12, 2021 17:11
satheon49 pushed a commit to satheon49/Cataclysm-DDA that referenced this pull request Aug 14, 2021
* better learning!
phase 1: Track highest xp ever reached, recover rust faster if you are below it, and also gain xp while you regain rust.
increment highestlevel if your highestexperience gets there.
* books only train theory
* rename "highest" to "knowledge" save confusion in long run
* recipe requirements can be theory based
* Books increase theory
and your ability to read a book is based on your theory level
* rust rate drops as you accumulate rust
* make book larnin' show theory experience
Run rust once per day, not at random times. Change the amount of xp lost instead of the frequency you do rust. It's a lot easier to calculate.
* book theory gain falls off as level gap increases
* NPC skill affects their teaching

Co-authored-by: actual-nh <[email protected]>
Co-authored-by: Kevin Granade <[email protected]>
@Malorn-Deslor
Copy link
Contributor

A bit late to the party, but I wanted to say how much I like this change.

I've just played roughly 30 hours with it over the past few days, and it really does feel like a fair system. I really didn't like the old system of skill rust, despite liking the concept of skills rusting. It just felt unfair, and insanely irritating when you were hovering around a skill level and kept losing it. I also disliked the fact that I could no longer try to make something because I had 'lost the ability' over the past five minutes.

This new system has addressed nearly all of these problems, allowing for meaningful progression and making books less overpowered as a happy side effect. It will be even better with the 24 hour skill rust pause, but I think it is in a pretty good place overall.

@l29ah
Copy link
Contributor

l29ah commented Oct 31, 2021

Do i get it right that theoretical level gives nothing except being able to attempt works on a vehicle, not even speed gain for practical experience, so reading non-mechanics books for skill is useless now?

@I-am-Erk
Copy link
Member Author

I-am-Erk commented Nov 2, 2021

Do i get it right that theoretical level gives nothing except being able to attempt works on a vehicle, not even speed gain for practical experience, so reading non-mechanics books for skill is useless now?

There's no reason mechanics should work any differently than other skills for speed gain.

@l29ah
Copy link
Contributor

l29ah commented Nov 2, 2021

It doesn't, and i'm not telling it should. I tell that theoretical knowledge gives nothing useful, so reading books is worthless. What speed gain are you talking about? I didn't notice any, nor could find it in the code.

@I-am-Erk
Copy link
Member Author

I-am-Erk commented Nov 2, 2021

This PR is literally that, what are you talking about? You can see it right in the changes of the very PR you are commenting on, as well as in the PR description above. I even commented it quite clearly in the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C++] Changes (can be) made in C++. Previously named `Code` <Enhancement / Feature> New features, or enhancements on existing Mechanics: Effects / Skills / Stats Effects / Skills / Stats PUBLIC TEST Come and try this!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants