Kruptein commented 7 months ago

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

develroo commented 6 months ago

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

Cheers!

develroo commented 6 months ago

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 commented 6 months ago

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 info@darragh.dev)

rexy712 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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

develroo commented 6 months ago

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 commented 6 months ago

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

develroo commented 6 months ago

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

sha256sum planar.sqlite 
35ed03fc5b2ebfadddbb09a205ac0c634d8253106a622635bd476020a40a6a19  planar.sqlite

Kruptein commented 6 months ago

Yeah got the same sha sum, it works correct here

develroo commented 6 months ago

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 <info@darragh.dev>
Date:   Fri Jan 5 20:42:07 2024 +0100

    chore: npm upgrade

Kruptein commented 6 months ago

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 commented 6 months ago

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

Kruptein commented 6 months ago

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 commented 6 months ago

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

develroo commented 6 months ago

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

Kruptein commented 6 months ago

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 commented 6 months ago

1343 uses older sqlite syntax

develroo commented 6 months ago

ok lemme revert back to 3.11-slim and try again

develroo commented 6 months ago

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 commented 6 months ago

Also can confirm the save file migration works perfectly for me

Kruptein / PlanarAlly

feat(Notes): Make notes more powerful #1336

History

Issues

The goal

Current progress

Todo or for later iteration

1343 uses older sqlite syntax