Cataclysm-BN icon indicating copy to clipboard operation
Cataclysm-BN copied to clipboard

feat(port): Add book binder with many QoL

Open leoCottret opened this issue 2 years ago • 4 comments

Summary

SUMMARY: Features "Add book binder with many QoL"

Purpose of change

Being able to copy and keep the most interesting recipes on you, even if they were above your level when you copied them. Also, it was one of the (many) PR to port before https://github.com/CleverRaven/Cataclysm-DDA/pull/46304 and https://github.com/CleverRaven/Cataclysm-DDA/pull/49408 (the final boss)

Describe the solution

Port https://github.com/CleverRaven/Cataclysm-DDA/pull/44399 and https://github.com/CleverRaven/Cataclysm-DDA/pull/50367

Changes from the original PR:

  • when you copy a new recipe, the old recipes that you already know are removed with a message. This means you'll be able to keep the same book binder for the entire game. image

  • the PR added some randomness to the spawn of paper, I didn't port it

  • you could only copy recipes that match your skill level, I removed the requirement

    • but instead, I added a check on the skill level when you open your craft menu
    • which means, you can copy any recipe from any book, but you will only be able to craft it when you met the requirement
    • I also removed the learnt recipes, and the recipes already in the book binder from the selection
    • hopefully, those changes will make it more user friendly
  • the pen can be used to write too (it has some charges now)

  • the copying time has been greatly reduced (it was quite long before, probably aligned with DDA pace)

  • lots of change to the book binder item

    • it's not a GENERIC item, but a TOOL
    • it uses paper as charges
    • by default (when you craft it, and when it spawns), it has respectively 0 or 1 charges (the first page is used)
    • then, copying a recipe increases the amount of charges in it
    • when it's full of charges, you can't add new recipes in this one
    • I could make it infinite, but since it's a tool and not a GENERIC item that contains paper, only the weight increases, not the volume. So I set the maximum at 500 pages (roughly 100 recipes). Although with the first point of this section, it doesn't matter much now.
    • lots of extra check have been added, or moved before the recipe to copy selection, to warn the player earlier that the action can't be done (cf tests)

Describe alternatives you've considered

Not porting it, and port the e-book one directly (but way too many changes) Porting the e-book one in this PR too (too big)

Testing

  • remove the flags UNLOAD, and RELOAD, spawn new character with 0 skills
  • test the following conditions: book is full, no paper on player, no writing tool, in the dark, you get special messages before selecting a recipe to copy
  • test the following conditions: 2 papers on the player, or "1 paper left" in the book: only the recipes with a number of page below or equal to those requirements can be selected (wasn't the case)
  • spawn the "A History of Firefighting" book
  • copy the fire axe and the crash axe recipe
  • the book has now 5 + 4 pages in it
  • open the copy recipe menu, they are not here anymore
  • drop the "A History of Firefighting" book, and go away from it
  • select view recipe, they are here
  • increase your fabrication skill to 5, try to craft them, they don't appear
  • increase your fabrication skill to 6, the crash axe appears
  • increase your fabrication skill to 9, the fire axe appears
  • drop the book binder, go away from it
  • they don't appear in the craft menu (not memorised yet)
  • read the "A History of Firefighting" book once
  • try to copy a recipe, then view the recipes. They disappeared because you learnt them by reading the book

leoCottret avatar Nov 06 '22 16:11 leoCottret

If it passes clang-tidy checks, this is ready for review

leoCottret avatar Nov 07 '22 01:11 leoCottret

that's great. one more idea: maybe it could also extract recipies from a book without requiring pen and paper, at the cost of destroying the book

scarf005 avatar Nov 07 '22 03:11 scarf005

that's great. one more idea: maybe it could also extract recipies from a book without requiring pen and paper, at the cost of destroying the book

Would it be worth it though? You would lose the ability to potentially read the book, for you and your followers. I asked a bit on discord what other people think, because I don't mind to add it if needed.

leoCottret avatar Nov 07 '22 11:11 leoCottret

/home/runner/work/Cataclysm-BN/Cataclysm-BN/src/iuse.cpp:9867:10: error: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty,-warnings-as-errors]
    if( !not_learnt_recipes.size() ) {
        ~^~~~~~~~~~~~~~~~~~~~~~~~~
        not_learnt_recipes.empty()

Coolthulhu avatar Nov 18 '22 20:11 Coolthulhu

is this PR still worked on?

Lamandus avatar Mar 26 '24 23:03 Lamandus

Same here, open it up again, if you are willing to work on it.

Lamandus avatar Apr 02 '24 15:04 Lamandus