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

removed number of chapters from abstract book template #40460

Closed
wants to merge 1 commit into from

Conversation

enaantd
Copy link
Contributor

@enaantd enaantd commented May 11, 2020

Summary

SUMMARY: Bugfixes "removed number of chapters from abstract book templates"

Purpose of change

Book templates had a fixed number of chapters, making it so that skill books had a limited number of uses, which is unwanted.

Describe the solution

I removed the number of chapters value from the abstract template.

Describe alternatives you've considered

None

Testing

I spawned a skill book in-game (Food Fashions for Young Moderns) and it had no number of uses, as opposed to previously.

Additional context

None

@ghost
Copy link

ghost commented May 11, 2020

This will make fun books added by #37377 that use that abstract have infinite chapters.

Example(western novel vs. Oxbridge Handbook of Mood Ailments, which is supposed to have 24 chapters):

image

@enaantd
Copy link
Contributor Author

enaantd commented May 11, 2020

I don't know which fun books should have a limited number of chapters and which shouldn't. I'd say all or none?

@kevingranade
Copy link
Member

All fun books should have a limited number of chapters.

@enaantd
Copy link
Contributor Author

enaantd commented May 13, 2020

I may be able to do this, but I'm unsure of several things and I think I'd benefit from some insights before starting a long and repetitive process.

Currently, many book templates are divided between fiction and non-fiction. I don't quite understand the need for this, and this can cause some problems. For instance, a scifi novel should be a fiction book (though in the cataclysm scifi may be reality, but we're talking about books written before the cataclysm), but a war novel may or may not be fiction. Yet, it makes sense to have a scifi novel and a war novel share the same template since they're, well, novels.

Unless there's something I missed, I think it would be opportune to change the separation in the templates from fiction/non-fiction to skill book/fun book. This would solve the number of chapters problem without having to edit each book's data or add even more inheriting templates. It may be out of scope for this PR and maybe way harder than I'd imagined, but I'm willing to take a look at this if my suggestion is appropriate.

@Jerimee
Copy link
Contributor

Jerimee commented May 21, 2020

I think the problem stems from the fact that some skill books grant fun. Presumably that fun should stop at some point, but the skill development (ie learning) shouldn't stop until max-learn level is achieved.

I'm not clear how this works, but you could look at fun_survival "Through the Lens" to figure it out. That books is fun: 3 and gets you to survival level 3. IIRC I've never used it up before getting to level 3, and I've never thought to read it indefinitely after level 3 to increase morale.

Book templates had a fixed number of chapters, making it so that skill books had a limited number of uses, which is unwanted.

This is true.

All fun books should have a limited number of chapters.

This is true.

I think it would be opportune to change the separation in the templates from fiction/non-fiction to skill book/fun book.

I don't necessarily disagree but...

Before I started messing with books, generally speaking all SKILL books were nonfiction and seemed hardbound. All FUN books were fiction and seemed paperback.

I wanted to break out of that rut without being too heavy handed. So the convention I created and am advocating for is (fiction or nonfict), then form (typically paperback or hardbound, though there are 6 or 7 forms in total), then subject, then short string id.

So a new book that is a hardbound "tragedy" novel should have the following id and copy from:

  • id: book_fict_hard_tragedy_blurk
  • copy-from: book_fict_hard_tragedy_tpl

That order might be less than ideal, but an order is better than ad hoc (which is more or less what we had just a couple months ago). Since merely organizing the books in the present order is time intensive, I'm doubtful that changing the order is a good use of time.

If you haven't already, look at itemgroups/SUS/library.json and items/book/abstract.json to see how I'm trying to organize them.

My intent is, with each abstract copying from the one above (until the actual book)

  1. book_nonf__tpl
  2. book_nonf_hard_tpl
  3. book_nonf_hard_cook_tpl
  4. food_fashions (an actual book)

book_nonf_hard_tpl should (arguably) have chapters, but skill books like those under book_nonf_hard_cook_tpl should not

There is chance that setting "chapters": 0 to book_nonf_hard_cook_tpl fixes the problem, but I'm just guessing. If that doesn't fix it, a hack might be to give book_nonf_hard_cook_tpl such a large number of chapters that it isn't an issue with regard to learning - but that doesn't resolve the issue with fun needing to be capped.

Not having a good answer at the moment, I'm tempted to suggest these admittedly cruddy solves:

  • leave it as is, even though the skill learn cap is unintentional
  • make it so no skill books are fun

Feel free to DM me on discord. Sorry for the long comment.

@anothersimulacrum anothersimulacrum added <Bugfix> This is a fix for a bug (or closes open issue) [JSON] Changes (can be) made in JSON labels May 29, 2020
@kevingranade
Copy link
Member

This is a symptom of the abstract being wrong, #37377 added a bunch of abstracts, redefined existing books in terms of those abstracts, and added new books. It introduced chapters to some of these books inappropriately. If you look at #37377 and restore the previous state of the pre-existing books, you're going to be 90% of the way there.

@Jerimee
Copy link
Contributor

Jerimee commented Jun 18, 2020 via email

@int-ua int-ua added the stale Closed for lack of activity, but still valid. label Dec 24, 2020
@stale
Copy link

stale bot commented Jan 24, 2021

This issue has been automatically closed due to lack of activity. This does not mean that we do not value the issue. Feel free to request that it be re-opened if you are going to actively work on it

@stale stale bot closed this Jan 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Bugfix> This is a fix for a bug (or closes open issue) [JSON] Changes (can be) made in JSON stale Closed for lack of activity, but still valid.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants