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

default: Improves reading and writing to books. #2656

Merged
merged 2 commits into from
Sep 4, 2021
Merged

Conversation

orbea
Copy link
Contributor

@orbea orbea commented Apr 21, 2020

Fixes #1743

This PR replaces #2646.

I prefer to do smaller and more concise PRs, but I am also not willing to indefinitely maintain multiple conflicting branches.

This does a few things:

  • Allow anyone to write to a book without any text and title.
  • Allows saving books without any text or title. (Saving book with erased contents does not work #1743)
  • A book without any title will be titled "Book".
  • Adds a "Read" and "Write" tab to written owned books.

This main goals behind this commit are to allow saving books without any text and/or title and to add a write/read tabs for owned books with text or title to make it easier for the owner to read.

Books without any text and title
empty

Owned books
write
read

Unowned books
unowned

* Allow anyone to write to a book without any text and title.
* Allows saving books without any text or title.
* A book without any title will be titled "Book".
* Adds a "Read" and "Write" tab to written owned books.

Fixes minetest#1743
@KaylebJay
Copy link

This PR is well done and should get some more attention. It has been more than a month.

@ghost
Copy link

ghost commented May 29, 2020

Instead of tabs, you could try a checkbox to toggle between reading and writing states.

@paramat paramat added the Feature label Aug 3, 2020
@paramat
Copy link
Contributor

paramat commented Aug 17, 2020

This has gone a while without core dev comment, so i want to add a comment to say this is appreciated. Unfortunately it seems we have been too busy to attend to this yet.
The bugfix part of this is of course supported.
Looking at the issue #1743 and previous PRs #2157 and #2646 there are some suggestions from non-core-devs for how books should behave, and unfortunately very little input from core devs on this.

Keeping in mind my lack of experience with MTG books, here are some questions. I may be missing something and there may be good reasons for your changes.

Allow anyone to write to a book without any text and title.

Perhaps a book without text and without title should just be default:book? In which case anyone will already be able to write to it.
Why would a book without text and without title, and able to be witten to by anyone, be anything other than default:book? Why make it different? How can it be different?

Allows saving books without any text or title

I am wondering why though? That is not a necessary part of the bugfix, the bug is: "Make a book with a bunch of random text in it's title and contents, save, then try erasing either field and save. When you open it again, you'll find that the field still has what you tried to erase."
One person commented: "it is a feature to prevent saving useless metadata to the book itemstack if no data is provided.", which seems to make sense.

The previous PR author asked: "One last question about the change, A written book with its content (title and text) deleted must be converted back to a book (unwritten)? Or is better leave it as a empty written book?"
Someone replied: "In my opinion, if a book has its content deleted, it becomes "unwritten" (fresh book).".
It makes more sense to me to revert it to default:book.

A book without any title will be titled "Book".

Seems good, if this is a book with text but no title.

Adds a "Read" and "Write" tab to written owned books.

Seems good.

This main goals behind this commit are to allow saving books without any text and/or title

Allowing saving if title and no text, or text and no title, seems good. But i am unsure about saving with neither of those.

Needs input from other core devs.

@orbea
Copy link
Contributor Author

orbea commented Aug 20, 2020

Fwiw I haven't actively played minetest in a while and I'm not sure I will spend any time making changes to this PR. I tested it thoroughly in my own world and found it worked for my purposes.

Perhaps a book without text and without title should just be default:book? In which case anyone will already be able to write to it.
Why would a book without text and without title, and able to be witten to by anyone, be anything other than default:book? Why make it different? How can it be different?

When I first used minetest it was remarked by another player how they could not save an empty book and that they did not like the false limitation. I don't think it causes any harm to be able to save a book without both text and title, it will still leave an author record which may or may not be fun for some players.

mods/default/craftitems.lua Outdated Show resolved Hide resolved
@sfan5 sfan5 merged commit 21e5f68 into minetest:master Sep 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Saving book with erased contents does not work
5 participants