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

feat(Notes): Make notes more powerful #1336

Merged
merged 54 commits into from
Jan 4, 2024
Merged

feat(Notes): Make notes more powerful #1336

merged 54 commits into from
Jan 4, 2024

Conversation

Kruptein
Copy link
Owner

@Kruptein Kruptein commented Dec 10, 2023

History

PA has 2 concepts of notes:

  • loose campaign notes
    • These have a title and some freeform text
    • Are always private
    • can be kept open / dragged around
  • shape notes
    • These have markdown aware freeform text and can be public or private
    • Limited to 1 per note
    • show on hover

Issues

There are multiple problems with the current implementation:

  • for shape notes
    • there is no way to have both private and public information
    • it would be nice to have alternatives to 'show on hover'
    • if you need the info you wrote for a shape, you have to find the actual shape wherever it may be
    • unless it's a character, notes are lost when the shape is removed
  • for loose notes
    • there is no way to search
    • there is no way to filter (e.g. only notes relevant for the current location)
    • there is no way to share these with other users
    • it would be nice if you could pin these to the map
    • only 1 note can be open at the same time

The goal

This PR attempts to fix all of these issues by unifying the current 2 note systems into 1 single system.

Current progress

A new big modal has been created: the note manager.
This will be the central place where you interact with notes.

This modal will list all the notes you have access to and allow you to manipulate them as well as pop them out so that they remain visible without the need to have the manager open.

Some completely new things:

  • You'll be able to tag notes for quick filtering.
  • pressing the n key, will open/close the note manager
  • You can specify if something is a campaign or a location global or a local note
    • After some thinking the concept of campaign/location/shape was too artificial and held some of the technical design back. If such a distinction is desired, users can use tags to simulate this
    • The note listing will by default hide notes not related to the active location
      • This can be changed using the search filters
    • Location notes will be pin-able to the map
      • The concept of location notes no longer exist, but being able to pin a note to the map is still planned, but maybe for a later iteration
  • The quick selection info on the right when you have a shape selected now has an extra notes bar
    • This can be used to quickly open all notes related to the shape
    • You can also hover over these to see the note contents and immediately go to popout/edit mode
  • You can now signal that a note icon should be shown on a shape.
    • This can be done as a reminder of an important note
    • This can be configured on a per note basis, if at least 1 of the attached notes has this setting, the icon will be shown
  • Notes attached to shapes will now list all shapes they are linked to
    • A note can be attached to multiple shapes due to templating (e.g. monster stat blocks)
    • The total number of linked shapes is shown
    • A detailed list of shapes in the current location is shown, you can click on them to move the camera

Things that changed:

  • You can have more than 1 pop out active at the same time
  • Popout note dialog has been restyled
    • It now by default shows the markdown rendered note
    • It can still be edited without going to the manager
    • It does offer an option to open the note in more details in the note manager (e.g. to configure settings / access)
  • It's become optional/configurable whether a shape's note should be shown on hover.
    • This will default to false
  • Note access is revamped
    • No longer binary private/public
    • grant default view/edit access
    • grant different access to specific users

Things that were removed:

  • The menu sidebar no longer shows notes, instead it acts as a button to open the note manager
  • The shape edit dialog no longer has a notes section.

Todo or for later iteration

  • Option to pin location notes to the map without having to first create a shape and put a note on it.
  • Configure trigger of note appearance (e.g. when visible, on hover, ...)
  • Configure location of note appearance (e.g. top of screen, above shape, popout, ...)
  • Decide what to do when a shape is removed with notes
  • Some user setting to allow html parsing in markdown
  • Allow reading notes in a completely separate tab
  • Provide note listing as a dedicated mobile page

@Kruptein Kruptein force-pushed the feature/improved-notes branch from f67903a to 2c48497 Compare December 10, 2023 12:46
@develroo
Copy link

Looks interesting. Be happy to test it when it lands.

Cheers!

@Kruptein Kruptein force-pushed the feature/improved-notes branch from 2c48497 to 927c231 Compare December 17, 2023 21:26
@Kruptein Kruptein marked this pull request as ready for review January 4, 2024 19:05
@Kruptein Kruptein merged commit 788a765 into dev Jan 4, 2024
5 checks passed
@Kruptein Kruptein deleted the feature/improved-notes branch January 4, 2024 19:09
@develroo
Copy link

develroo commented Jan 4, 2024

Just tried using the dev build but it is failing to upgrade the DB version.

KeyError: '49fab00e-729d-4629-aa83-953e529b762e'
2024-01-04 19:20:32,577 - ERROR - ERROR: Could not start server (save.py:611)
2024-01-04 19:21:21,177 - WARNING - Save format 88 does not match the required version 89! (save.py:591)
2024-01-04 19:21:21,178 - WARNING - Attempting upgrade (save.py:594)
2024-01-04 19:21:21,179 - WARNING - Backing up old save as /planarally/save_backups/planar.sqlite.88 (save.py:624)
2024-01-04 19:21:21,186 - WARNING - Starting upgrade to 89 (save.py:603)
2024-01-04 19:21:21,205 - ERROR - '49fab00e-729d-4629-aa83-953e529b762e' (save.py:607)
Traceback (most recent call last):
  File "/planarally/src/save.py", line 605, in upgrade_save
    upgrade(db, save_version)
  File "/planarally/src/save.py", line 556, in upgrade
    note_id = shape_to_note_ids[shape_id]
              ~~~~~~~~~~~~~~~~~^^^^^^^^^^
KeyError: '49fab00e-729d-4629-aa83-953e529b762e'
2024-01-04 19:21:21,208 - ERROR - ERROR: Could not start server (save.py:611)

Thoughts?

Edit: FWIW I know I had some notes already made in some games.

@Kruptein
Copy link
Owner Author

Kruptein commented Jan 4, 2024

Just tried using the dev build but it is failing to upgrade the DB version.

KeyError: '49fab00e-729d-4629-aa83-953e529b762e'
2024-01-04 19:20:32,577 - ERROR - ERROR: Could not start server (save.py:611)
2024-01-04 19:21:21,177 - WARNING - Save format 88 does not match the required version 89! (save.py:591)
2024-01-04 19:21:21,178 - WARNING - Attempting upgrade (save.py:594)
2024-01-04 19:21:21,179 - WARNING - Backing up old save as /planarally/save_backups/planar.sqlite.88 (save.py:624)
2024-01-04 19:21:21,186 - WARNING - Starting upgrade to 89 (save.py:603)
2024-01-04 19:21:21,205 - ERROR - '49fab00e-729d-4629-aa83-953e529b762e' (save.py:607)
Traceback (most recent call last):
  File "/planarally/src/save.py", line 605, in upgrade_save
    upgrade(db, save_version)
  File "/planarally/src/save.py", line 556, in upgrade
    note_id = shape_to_note_ids[shape_id]
              ~~~~~~~~~~~~~~~~~^^^^^^^^^^
KeyError: '49fab00e-729d-4629-aa83-953e529b762e'
2024-01-04 19:21:21,208 - ERROR - ERROR: Could not start server (save.py:611)

Thoughts?

Edit: FWIW I know I had some notes already made in some games.

Can you share your sqlite file with me on dc? (edit: or mail it to me [email protected])

@rexy712
Copy link
Contributor

rexy712 commented Jan 4, 2024

I also have an error during save file upgrade

2024-01-04 14:52:12,363 - WARNING - Save format 88 does not match the required version 89! (save.py:591)
2024-01-04 14:52:12,363 - WARNING - Attempting upgrade (save.py:594)
2024-01-04 14:52:12,363 - WARNING - Backing up old save as /planarally/server/save_backups/planar.sqlite.88 (save.py:624)
2024-01-04 14:52:12,365 - WARNING - Starting upgrade to 89 (save.py:603)
2024-01-04 14:52:12,366 - ERROR - 'annotation_visible' (save.py:607)
Traceback (most recent call last):
  File "/planarally/server/src/save.py", line 605, in upgrade_save
    upgrade(db, save_version)
  File "/planarally/server/src/save.py", line 499, in upgrade
    del template["annotation_visible"]
        ~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^
KeyError: 'annotation_visible'
2024-01-04 14:52:12,367 - ERROR - ERROR: Could not start server (save.py:611)

I will also send the sqlite file via email if needed

@Kruptein
Copy link
Owner Author

Kruptein commented Jan 4, 2024

I also have an error during save file upgrade

2024-01-04 14:52:12,363 - WARNING - Save format 88 does not match the required version 89! (save.py:591)
2024-01-04 14:52:12,363 - WARNING - Attempting upgrade (save.py:594)
2024-01-04 14:52:12,363 - WARNING - Backing up old save as /planarally/server/save_backups/planar.sqlite.88 (save.py:624)
2024-01-04 14:52:12,365 - WARNING - Starting upgrade to 89 (save.py:603)
2024-01-04 14:52:12,366 - ERROR - 'annotation_visible' (save.py:607)
Traceback (most recent call last):
  File "/planarally/server/src/save.py", line 605, in upgrade_save
    upgrade(db, save_version)
  File "/planarally/server/src/save.py", line 499, in upgrade
    del template["annotation_visible"]
        ~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^
KeyError: 'annotation_visible'
2024-01-04 14:52:12,367 - ERROR - ERROR: Could not start server (save.py:611)

I will also send the sqlite file via email if needed

Thanks! this is even a different error. nice to have some dev testers I suppose :D
sorry for the breakage.

@Kruptein
Copy link
Owner Author

Kruptein commented Jan 5, 2024

This has been addressed in #1341 and was merged to dev.

@develroo
Copy link

develroo commented Jan 5, 2024

This has been addressed in #1341 and was merged to dev.

Ok a new error has occurred.

2024-01-05 22:04:22,884 - WARNING - Save format 88 does not match the required version 89! (save.py:634)
2024-01-05 22:04:22,884 - WARNING - Attempting upgrade (save.py:637)
2024-01-05 22:04:22,885 - WARNING - Backing up old save as /planarally/save_backups/planar.sqlite.88 (save.py:667)
2024-01-05 22:04:22,902 - WARNING - Starting upgrade to 89 (save.py:646)
2024-01-05 22:04:22,918 - ERROR - near "DROP": syntax error (save.py:650)
Traceback (most recent call last):
  File "/usr/local/lib/python3.11/site-packages/peewee.py", line 3252, in execute_sql
    cursor.execute(sql, params or ())
sqlite3.OperationalError: near "DROP": syntax error

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/planarally/src/save.py", line 648, in upgrade_save
    upgrade(db, save_version)
  File "/planarally/src/save.py", line 605, in upgrade
    db.execute_sql("ALTER TABLE shape DROP COLUMN annotation")
  File "/usr/local/lib/python3.11/site-packages/peewee.py", line 3250, in execute_sql
    with __exception_wrapper__:
  File "/usr/local/lib/python3.11/site-packages/peewee.py", line 3020, in __exit__
    reraise(new_type, new_type(exc_value, *exc_args), traceback)
  File "/usr/local/lib/python3.11/site-packages/peewee.py", line 192, in reraise
    raise value.with_traceback(tb)
  File "/usr/local/lib/python3.11/site-packages/peewee.py", line 3252, in execute_sql
    cursor.execute(sql, params or ())
peewee.OperationalError: near "DROP": syntax error
2024-01-05 22:04:22,925 - ERROR - ERROR: Could not start server (save.py:654)

@Kruptein
Copy link
Owner Author

Kruptein commented Jan 5, 2024

Are you sure your save file is still in tact, it works fine for me with the file you sent me on discord

@develroo
Copy link

develroo commented Jan 5, 2024

Well I sent you the backup so yeah I assume it is the same.

sha256sum planar.sqlite 
35ed03fc5b2ebfadddbb09a205ac0c634d8253106a622635bd476020a40a6a19  planar.sqlite

@Kruptein
Copy link
Owner Author

Kruptein commented Jan 5, 2024

Yeah got the same sha sum, it works correct here

@develroo
Copy link

develroo commented Jan 5, 2024

Hmm. Odd then? What could be different. I am building the docker image from the git repo so ?

Edit:

This is the right commit, yes?

commit 6032d916b072fbf52d2f992b1e740f72a3fdc28b (HEAD -> dev, origin/dev, origin/HEAD)
Author: Darragh Van Tichelen <[email protected]>
Date:   Fri Jan 5 20:42:07 2024 +0100

    chore: npm upgrade

@Kruptein
Copy link
Owner Author

Kruptein commented Jan 5, 2024

I guess somehow the sqlite in the docker container is older than my local sqlite. The syntax it's failing on is from 2021-03-12 (3.35.0)

@develroo
Copy link

develroo commented Jan 5, 2024

Let me check the Dockerfile see if there is a newer base image

@Kruptein
Copy link
Owner Author

Kruptein commented Jan 5, 2024

yeah it has 3.34.1-3 I believe, I could move the docker image to python 3.12 which is based on debian 12 instead of 11 and has a newer sqlite version

@Kruptein
Copy link
Owner Author

Kruptein commented Jan 5, 2024

I guess I shall rewrite the drop column commands to an older syntax just to be less breaking for other setups

@develroo
Copy link

develroo commented Jan 5, 2024

Let me bump the base image to 21.5-alpine. See if that fixes it.

@Kruptein
Copy link
Owner Author

Kruptein commented Jan 5, 2024

Let me bump the base image to 21.5-alpine. See if that fixes it.

it won't because that's the image for the frontend which is not relevant. the server runs in the python:3.11-slim image

@Kruptein
Copy link
Owner Author

Kruptein commented Jan 5, 2024

#1343 uses older sqlite syntax

@develroo
Copy link

develroo commented Jan 5, 2024

ok lemme revert back to 3.11-slim and try again

@develroo
Copy link

develroo commented Jan 5, 2024

Woot! OK that seems to fix it. But might be worth making a note if the python versions bumps and breaks because of deprecated syntax.

2024-01-05 22:49:59,920 - WARNING - Save format 88 does not match the required version 89! (save.py:641)
2024-01-05 22:49:59,920 - WARNING - Attempting upgrade (save.py:644)
2024-01-05 22:49:59,921 - WARNING - Backing up old save as /planarally/save_backups/planar.sqlite.88 (save.py:674)
2024-01-05 22:49:59,941 - WARNING - Starting upgrade to 89 (save.py:653)
2024-01-05 22:49:59,985 - WARNING - Upgrade to 89 done. (save.py:664)
2024-01-05 22:49:59,986 - WARNING - Upgrade process completed successfully. (save.py:666)

======== Starting Webserver on https://0.0.0.0:8000/ ========

======== Starting APIserver on https://0.0.0.0:8010/ ========

(Press CTRL+C to quit)

@rexy712
Copy link
Contributor

rexy712 commented Jan 5, 2024

Also can confirm the save file migration works perfectly for me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants