Adds returning to last read page for journals #1679
Conversation
| ptVault().addChronicleEntry(kBookChronicle, 0, temp) | ||
|
|
||
| def EncodeLocPath(self): | ||
| temp = zlib.adler32(LocPath.value.encode('utf-8')) & 0xffffffff |
There was a problem hiding this comment.
| temp = zlib.adler32(LocPath.value.encode('utf-8')) & 0xffffffff | |
| temp = zlib.adler32(LocPath.value.encode('utf-8')) |
zlib.adler32 is guaranteed to return a 32-bit integer, so we shouldn't need to mask to 32-bits
| temp = json.dumps(BookJson) | ||
| ptVault().addChronicleEntry(kBookChronicle, 0, temp) | ||
|
|
||
| def EncodeLocPath(self): |
There was a problem hiding this comment.
This would be a good place to use a @property instead of a function, IMO.
| else: | ||
| BookJson.pop(self.EncodeLocPath(), None) | ||
| temp = json.dumps(BookJson) | ||
| while len(temp) > 1024: |
There was a problem hiding this comment.
A comment for posterity about why this limitation is needed would be nice.
| BookJson[self.EncodeLocPath()] = JournalBook.getCurrentPage() | ||
| else: | ||
| BookJson.pop(self.EncodeLocPath(), None) | ||
| temp = json.dumps(BookJson) |
There was a problem hiding this comment.
| temp = json.dumps(BookJson) | |
| temp = json.dumps(BookJson, separators=(",", ":"))) |
This will compact the json more by eliminating whitespace.
| JournalBook = None | ||
| BookJson = None | ||
| CurrentPage = -1 |
There was a problem hiding this comment.
Could we do this without adding more globals? Instead, prefer instance members.
| JournalBook.open(CurrentPage) #Opens to saved page | ||
| JournalBook.goToPage(CurrentPage) #shows page tabs at bottom when used after open |
There was a problem hiding this comment.
Looks like we may need to fix a bug in pfJournalBook. Is it that the page turn buttons are missing?
There was a problem hiding this comment.
Yes, during my testing both commands were needed to consistently open the book to the correct page and show the tabs.
| BookJson = ptVault().findChronicleEntry(kBookChronicle) | ||
| if BookJson: | ||
| BookJson = json.loads(BookJson.getValue()) | ||
| else: | ||
| BookJson = {} |
There was a problem hiding this comment.
We probably shouldn't be hanging onto the value of the chronicle, in case some rogue vault manager comes along and changes the value of the chronicle while we're reading the journal. This would cause the vault manager's change to be discarded.
|
A few general thoughts about this approach.... JSON is a bit much for this, I think, but OTOH it's definitely got less boilerplate required for getting started. We might consider just making a chronicle for each loc path under a parent journal chronicle, that way we don't have to worry about the character limits and can forget about the whole JSON thing. I am a little concerned about how this might work for journals in Ages that are being updated. Cyan's journals are unlikely to change, so that's not a big deal. But, it might be a little surprising if you're in a certain fan Age on page 100 of the journal, and the author suddenly rewrites a significant part of the first 100 pages, meaning you open the journal up to something very different. Also, we'll want to test what happens if we try to open to page 100 if there are only 98 pages (pages get deleted).
That sounds like a bug we need to fix, IMO. |
|
I dont know if its possible but checking the max pages and if the current page is greater than max, go back to zero or cover |
FWIW, JSON was suggested for this to avoid making a bunch of Chronicles that all need to be fetched at login as part of the player Vault tree. The concern was that it could potentially lead to SetActivePlayer timeouts (although the Chronicles would be small so it's probably not likely, but a single JSON blob is less data to look up and transfer than a hundred single-int Chronicles). |
https://youtu.be/A0GeF4ayUkA
This saves the last 68 journals read pages to a player chronicle and will open to that page when you open the journal again.
68 journals is how many alder32 hashed strings you can fit in a 1024 character json string
1024 is character limit decided on due to this issue
2048 is probably the string limit for other vault implementations
https://youtu.be/A0GeF4ayUkA?t=27
The only issue I found during testing has to do with the cover and first page of the journal.
There is currently no notify message when you turn from the cover to the first page.
Due to this I coded after the book kNotifyShow is received, it sets the page to current page (zero).
If you open the book but dont turn any pages and leave, the next time you open the book it will open the book to page 0 instead of the cover.
Operations tested with new code:
I tested these also closing client and entering age again as well
Also tested journals that don't have a cover (Serene Journal)
Cover -> First Page -> Close = First Page
Cover -> First Page -> Cover = Cover
Cover -> Close = First Page (Only issue found so far)
Any Page -> Close = Last used page
Any Page -> Cover -> Close = Cover