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

Chapters are Character-mapped for unique books #79388

Merged

Conversation

ShnitzelX2
Copy link
Contributor

Summary

Bugfixes "chapters are character-mapped for unique books"

Purpose of change

Closes #77920

As explained in above issue, books can be copied indefinitely for infinite fun.

Describe the solution

Chapters read for each book itype are mapped per-Character instead of per-item unless "generic": true is added to a book specifying it as a type of book rather than a specific title (e.g. book of essays)

Describe alternatives you've considered

Testing

Read a copy of Dune, noted that another copy had same "chapters read"
Read a (generic = true) Playboy until all the charges were gone, noted a new copy had 4 chapters still left

Read a manual to make sure it still worked

Additional context

Not that it explains much, but you can see the copies of Dune I've read have the same chapters read and the other generic books don't:
image

@github-actions github-actions bot added [JSON] Changes (can be) made in JSON [C++] Changes (can be) made in C++. Previously named `Code` <Bugfix> This is a fix for a bug (or closes open issue) json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Jan 28, 2025
Copy link
Contributor

@moxian moxian left a comment

Choose a reason for hiding this comment

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

Please document the newly added generic field in .md somewhere

More importantly, I'm not very familiar with our save handling, but the lack of explicit de-/serialization of the newly added Character::book_chapters makes me think that this info would be lost after a save-load cycle? Can you please double-check that this part works?

src/character.h Outdated Show resolved Hide resolved
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jan 28, 2025
@Procyonae
Copy link
Contributor

Doesn't this still result in the linked issue being present for generic books?

@ShnitzelX2 ShnitzelX2 force-pushed the character-map-book-chapters branch from 34b07d3 to 1b8270a Compare January 28, 2025 22:24
@github-actions github-actions bot added <Documentation> Design documents, internal info, guides and help. [Markdown] Markdown issues and PRs labels Jan 28, 2025
@ShnitzelX2 ShnitzelX2 force-pushed the character-map-book-chapters branch from 1b8270a to 4e592a5 Compare January 28, 2025 22:35
@ShnitzelX2
Copy link
Contributor Author

So I've fixed:

document the newly added generic field in .md

de-/serialization of the newly added Character::book_chapters

marked as mutable

please add a short comment noting that it represents the amount of chapters remaining in a given book

linked issue being present for generic books

Great reviews, thanks. Double-check if you'd like.

Copy link
Contributor

@moxian moxian left a comment

Choose a reason for hiding this comment

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

Uh.. do i need to press this approve button here? Still not quite sure how github reviews work, apologies

-instead of item instances mapping character->chapter count
-"generic" books keep original chapter read behavior and cannot be copied, books with chapters now can
@ShnitzelX2 ShnitzelX2 force-pushed the character-map-book-chapters branch from 4e592a5 to c21b37d Compare January 29, 2025 01:33
@Maleclypse Maleclypse merged commit e02f107 into CleverRaven:master Jan 29, 2025
27 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` <Documentation> Design documents, internal info, guides and help. [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions [Markdown] Markdown issues and PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Books stored in smartphone are new instances of the book type
4 participants