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

Adds public notes #28373

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

Contrabang
Copy link
Contributor

@Contrabang Contrabang commented Feb 13, 2025

What Does This PR Do

Adds public notes. Old notes are grandfathered in as private notes. Upon creation of a new note, admins will be able to choose if its public or private. Admins can toggle notes created by ONLY THEM between private and public. Only you can see your own public notes.

Why It's Good For The Game

Admins want it as an option. Its also helpful and healthy for the playerbase.

Testing

A lot of testing. Including setting up a DB. (ignore that I'm a headcoder and haven't done it yet.)
Player view.
image
Admin view.
image

Edit: The player-side is darkmode now!
image


Declaration

  • I confirm that I either do not require pre-approval for this PR, or I have obtained such approval and have included a screenshot to demonstrate this below.

Changelog

🆑
add: Public player notes. View these with the "Show My Notes" OOC verb.

@Contrabang Contrabang added the Administration This PR relates to ingame administration features label Feb 13, 2025
@@ -1,4 +1,4 @@
/proc/add_note(target_ckey, notetext, timestamp, adminckey, logged = 1, checkrights = 1, show_after = TRUE, automated = FALSE, sanitise_html = TRUE) // Dont you EVER disable this last param unless you know what you're doing
Copy link
Member

Choose a reason for hiding this comment

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

Edit comment to say automated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment changed to // Dont you EVER disable 'sanitise_html', only automated systems should use this

Comment on lines +289 to +293
var/target_ckey = query_find_note_edit.item[1]
var/text = query_find_note_edit.item[2]
var/adminckey = query_find_note_edit.item[3]
var/automated = query_find_note_edit.item[4]
var/public = query_find_note_edit.item[5]
Copy link
Member

Choose a reason for hiding this comment

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

We don't need the text or target ckey here surely right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its used in the logging.

@ParadiseSS13-Bot ParadiseSS13-Bot added -Status: Awaiting type assignment This PR is waiting for its type to be assigned internally SQL Change This PR modifies the game database. This PR must go through the host. Configuration Change This PR changes the game configuration files. Please run via the host. labels Feb 13, 2025
@ParadiseSS13-Bot ParadiseSS13-Bot added -Status: Awaiting review This PR is awaiting review from the review team and removed -Status: Awaiting type assignment This PR is waiting for its type to be assigned internally labels Feb 14, 2025
@Bm0n
Copy link
Contributor

Bm0n commented Feb 21, 2025

I don't know if "public notes" is the right wording to convey what's being done.

Public would infer that everyone can see your notes when in reality it's between the admins and the player who was noted.

Visible notes instead? Or forgo the word public entirely and call the panel "my notes"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-Status: Awaiting review This PR is awaiting review from the review team Administration This PR relates to ingame administration features Configuration Change This PR changes the game configuration files. Please run via the host. SQL Change This PR modifies the game database. This PR must go through the host.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants