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

Update oc_core.lsl for wearer setting of safeword and device name #999

Merged
merged 4 commits into from
Jul 28, 2024

Conversation

Medea-Destiny
Copy link
Collaborator

Fixes for #986 and #987. This version of oc_core allows wearer to change own safeword and device name when not owner. Fix includes improvements to notifications to avoid silent authorization failures.

REQUIRES IN-WORLD TESTING before merge:
Wearer should now be able to change safeword and device name, as well as owner.
Wearer should not be able to disable safeword, only owner.
Wearer should now be notified if safeword is disabled or if device name or safeword is changed.
"(prefix) safeword" should inform wearer or owner of current safeword.
"(prefix) device name" should inform wearer or owner of current device name.
User who is neither wearer nor owner should get clear no access notification for safeword or device commands rather than silent failure.

Notes:
Refactor of safeword function in usercommand. 'Safeword off' now no longer sets safeword to 'off' before disabling, resulting in confusing "Safeword is now set to 'off'" message. Instead safeword off is clearly notified. Wearer can now set their own safeword, but only owners can disable it still. See issue # 986. Attempting to access safeword without permission now gives no access response.
Provide no access notification for device name, and allow non-owner wearer to name. Notify wearer as well when another person changes device name. See issue # 987

@Medea-Destiny Medea-Destiny added Do Not Merge (yet) One reason or another for not merging 8.3 Target version 8.,3 labels Oct 22, 2023
@Pingout
Copy link
Collaborator

Pingout commented Oct 23, 2023

We need you to look at this again Medea. It's doing about half of what we expected.
The [prefix] safeword off command now returns a notification that the safeword has been disabled. As expected.
The problem is that the non-owner collar wearer can still just turn the safeword back on by using: [prefix] safeword RED (or whatever) and the safeword is no longer disabled. I think if the Collar Owner has disabled the Safeword, the collar Wearer should get a fail notice when attempting to turn the safeword back on.

@NikkiLacrima
Copy link
Contributor

NikkiLacrima commented Nov 14, 2023

"prefix safeword-enable/safeword-disable" in oc-api handles safeword enabling and disabling. So doing "prefix safeword off" in oc_core is not necessary, and we can let oc_core manage only the setting of new safewords and leave the enable/disable functionality to oc_api.

oc_api doesnt fully read and save the global_safeworddisable and as seen in oc_core (around line 797) the way used to reenable safeword is to send llMessageLinked(LINK_SET, CMD_OWNER, "safeword-enable",""); This section should further be removed so setting safeword to RED does not reenable the safeword.

If we want prefix safeword off to still work (duplicating prefix safeword-enable) it can simply call llMessageLinked(LINK_SET, CMD_OWNER, "safeword-enable","");

So questions for the usage are:

  • Do we want safeword to be automatically enabled when owner sets a new safeword, or should prefix safeword-enable be necessary
  • do we want both prefix safeword off and prefix safeword-disable

@Medea-Destiny
Copy link
Collaborator Author

@NikkiLacrima I've got a fix already handed to Ping inworld for testing which simply rejects setting a new safeword by non-owner authed wearer when safeworddisable is on, which strikes me as the simple fix.

the safeword-disable and safeword-enable commands handled in oc_api are triggered by oc_core if the global_safeword setting is RED as a way to reset, but there are problems with this method. oc_api doesn't store changes by safeword-disable to settings, and it doesn't clear the safeword setting itself, so help/about will show your safeword with no indication that it's non-functional, while (prefix) safeword off will set the global_safeworddisable setting and change the safeword to "off" so that Help/About shows "Safeword: off"

I'm not sure if the safeword-enable function is used in any other way, but I don't think so. I can't find any other instance of safeword-enable LMs, and no instances of safeword-disable. I think if we change the safeword-enable trigger in oc_core when global_safeword is RED to deleting the global_safeworddissable setting we'll get the same effect, avoid the incorrect display of safeword, avoid oc_api getting out of sync with global settings, and save a few lines in oc_api. Though maybe we shouldn't have the safeword disable setting changed unless it's explicitly changed by owner, and should instead enforce safeword to "off" when safewworddisable is true.

Thoughts?

@NikkiLacrima
Copy link
Contributor

NikkiLacrima commented Nov 18, 2023

In short remove the command strings "safeword-disable" and "safeword-enable" and instead handle the global_safeworddissable setting both when set and removed, that sounds like the best option, and more consistent with how we manage state using settings. Using safeword off to set the safeword to "off" or "(off)" and global_safeworddissable = TRUE seems good.

One more thing, found while working on the runaway stuff: on oc_core line about 220

``

if (iNum == CMD_OWNER && sStr == "runaway") {
    llMessageLinked(LINK_SET, LM_SETTING_DELETE, "auth_owner","origin");
    llMessageLinked(LINK_SET, LM_SETTING_DELETE, "auth_trust","origin");
    llMessageLinked(LINK_SET, LM_SETTING_DELETE, "auth_block","origin");
    return;
} 

``
This is a bug allowing owners to be deleted without confirming a runaway. Can you remove this section of code in your patch, it is handled in oc_api.

@Pingout
Copy link
Collaborator

Pingout commented Nov 19, 2023

As tested, with Medea's update the Safeword problem shown above is solved. Once the Safeword is disabled by the collar Owner, the collar Wearer can no longer turn the Safeword function back on by simply assigning a new Safeword. When the Wearer attempts this they get a notice "Only Owners can set a safeword when disabled!"
The Owner can turn the Safeword function on by assigning a new Safeword using [prefix] safeword [newsafeword]. This will remove the global=safeworddisable~1 setting
The collar Wearer is then able to assign their own Safeword using [prefix] safeword [newsafeword]

@Pingout
Copy link
Collaborator

Pingout commented Nov 29, 2023

Through various testing scenarios we have uncovered a problem with the latest oc_core update.
The fix does allow the Wearer to rename their own safeword
The fix allows the Owner to disable the safeword using [prefix] safeword off
The Wearer then gets a notice when trying to change the safeword saying "Only Owners can set a safeword when disabled!"
The problem comes when the Owner turns the safeword back on using [prefix] safeword [newsafeword]
If the Owner sets the safeword to BLUE, for example, the collar sends a notice saying the safeword is now BLUE. However the safeword remains disabled. If the Wearer attempts to use the safeword BLUE, the command will fail silently. If the Wearer attempts to set a new safeword, that attempt will also fail silently.
But if the Owner uses the command [prefix] safeword RED, the safeword is enabled, and the Wearer is once again allowed to use or change the safeword.
The script may be only listening for the specific safeword RED from the Owner

@Medea-Destiny
Copy link
Collaborator Author

So, it looks like OC_API does not set g_iSafewordDisable to 0 when the global_safeworddisable setting is removed, only the reverse. Which is an odd behaviour honestly. I've added a safeword-enable link message when a new safeword is set, which should clear the issue @Pingout discovered up. A more rational arrangement can be done of this kind of situation where a variable is shared by two scripts when we move it over to LSD.

The updated version of the script already has the #1054 changes merged into it for ease of testing. Assuming this one is good we won't need to do the separate merge for #1054

@Medea-Destiny Medea-Destiny added the Needs testing This issue needs volunteers to try to duplicate the error or identify a caues label Jul 13, 2024
@Medea-Destiny
Copy link
Collaborator Author

In-world testing provided by Kristina Hearts:

*Wearer should now be able to change safeword and device name, as well as owner.

changed safeword to "blue"
/1 kr safeword blue
status change was successful
[11:07] 7.2024 oc_core beta testing: Safeword is now set to 'blue'.
􀀁

changed back to "red" successfully
[11:10] 7.2024 oc_core beta testing: Safeword is now set to 'red'.
􀀀

added myself as owner of my collar successfully
[11:11] 7.2024 oc_core beta testing: You are now a owner on this collar

changed devise name successfully, notes below .

[11:17] 7.2024 oc_core beta testing: OpenCollar Settings:
[11:17] 7.2024 oc_core beta testing: settings=nocomma1
global=checkboxes
▢,▣
global=safewordred
capture=status
8
auth=owner~ca0da0bf-7941-4c6d-b966-893afce71ad8

ran command
/1 kr add owner 26ac30da-6625-44e7-8338-7d8192c85f18
[11:22] 7.2024 oc_core beta testing: RebeccaDeLucy has been added as owner
successful

removed owner
[11:26] 7.2024 oc_core beta testing: RebeccaDeLucy has been removed from the owner role
successful

added owner
/1 kr add owner 26ac30da-6625-44e7-8338-7d8192c85f18
[11:28] 7.2024 oc_core beta testing: RebeccaDeLucy has been added as owner
change safeword to "blue"
[11:29] 7.2024 oc_core beta testing: Safeword is now set to 'blue'.
tested safeword
/1 blue
[11:31] 7.2024 oc_core beta testing: You used the safeword, your owners have been notified
[11:31] 7.2024 oc_core beta testing: Kristina Hearts (kristinahearts) had to use the safeword. Please check on Kristina Hearts (kristinahearts).
[11:31] 7.2024 oc_core beta testing: Relay temporarily suppressed for 30 seconds due to safeword or clear all.
[11:31] May O. Mingzi (mayomingzi) is offline.
[11:31] 7.2024 oc_core beta testing: Relay settings have been restored.
[11:31] 7.2024 oc_core beta testing: Relay temporarily suppressed for 30 seconds due to safeword or clear all.
[11:32] 7.2024 oc_core beta testing: Relay settings have been restored.
Successful

================================================================================
*Wearer should not be able to disable safeword, only owner.

Added 2nd owner
[11:56] 7.2024 oc_core beta testing: Trinity Lovette (chrispeterson) has been added as owner
Successful

commands run by Trinity
Owner Trinity Lovette removed myself as owner
[11:58] 7.2024 oc_core beta testing: Kristina Hearts (kristinahearts) has been removed from the owner role
[11:58] 7.2024 oc_core beta testing: You have been removed from Kristina Hearts (kristinahearts)'s collar
/1 kr safeword abcd

Commands run by collar wearer Kristina
[12:01] 7.2024 oc_core beta testing: Safeword is now set to 'abcd'.
/1 kr safeword off
[12:04] OpenCollar Stock Collar Beta Testing 2: Only an owner can disable Safeword!

Commands run by Trinity
/1 kr safeword off
[12:08] 7.2024 oc_core beta testing: Safeword Disabled.
/1 kr safeword on
[12:09] 7.2024 oc_core beta testing: Safeword is now set to 'on'.
/1 kr safeword 12345
[12:09] 7.2024 oc_core beta testing: Safeword is now set to '12345'.
[12:11] 7.2024 oc_core beta testing: OpenCollar Settings:
[12:11] 7.2024 oc_core beta testing: settings=nocomma~1

checked settings:
global=checkboxes▢,▣
global=safeword
12345
capture=status8
auth=owner
26ac30da-6625-44e7-8338-7d8192c85f18,5f8f9e1e-17bc-4284-a445-81f0bec6bc9e
rlvsuite=masks~0,0

SUCCESSFUL

===============================================================================================
*Wearer should now be notified if safeword is disabled or if device name or safeword is changed.

Command ran by Trinity against Kristina's collar:
/1 kr safeword 246
[12:20] 7.2024 oc_core beta testing: Safeword is now set to '246'.
Nearby chat of Kristina Hearts
[12:20] 7.2024 oc_core beta testing: Safeword is now set to '246'.
SUCCESSFUL

Command ran by Trnity against Kristina's collar:
/1 kr safeword off
[12:29] 7.2024 oc_core beta testing: Safeword Disabled.
/1 kr safeword on
[12:30] 7.2024 oc_core beta testing: Safeword is now set to 'on'.

Kristina Hearts Nearby Chat:
[12:29] 7.2024 oc_core beta testing: Safeword Disabled.
[12:30] 7.2024 oc_core beta testing: Safeword is now set to 'on'.
Successful

Command ran by Trnity against Kristina's collar
/1 kr name fixme
[12:23] 7.2024 oc_core beta testing: The wearer's name is now set to fixme (if this is the old name, please type '/1 (prefix) name' to confirm the change went through, we may just have lagged)

Kristina Nearby Chat shows no notification that there has been a name change
FAILURE

========================================================================
*"(prefix) safeword" should inform wearer or owner of current safeword.

When safeword is turned "ON" by Trinity
Both Kristina (collar wearer) and Trinity (owner) ran the following
/1 safeword
[13:42] Boston: Safeword is now set to 'on'.

When safewood is turned "OFF" by Trinity
/1 kr safeword off
/1 kr safeword
[12:48] 7.2024 oc_core beta testing: The safeword is current set to: 'on'

When Kristina check her collar's status after having safewords turned off
/1 kr safeword
[12:50] 7.2024 oc_core beta testing: The safeword is current set to: 'on'

It does not inform the wearer or owner of what the safeword is but it does
inform them that status which may be incorrect.
This is a possible failure if looking for what's the current safeword and
not the status of the safeword and it's true on/off status.
FAILURE

========================================================================
*"(prefix) device name" should inform wearer or owner of current device name.

both the wearer and owner ran the following command
/1 kr device name
[13:14] 7.2024 oc_core beta testing: The current device name is: 7.2024 oc_core beta testing
SUCCESSFUL

========================================================================
*When changing name or device name, the correct new name should be given, i.e, "The Wearer's name is now set to (newname)"

ran as follows
[prefix] device name [newname] changes the name of the collar in menus and chat messages.
This did not work
**error in the syntax from above which I ha taken from as show on the web

it should be just:
[prefix] device [newname]
ran as following
/1 kr device Boston
[13:22] Boston: The device name is now set to: Boston (if this is the old name, please type '/1 (prefix) device name' to confirm the change went through, we may just have lagged)
[13:22] Boston: The current device name is: Boston
SUCCESSFUL

========================================================================

  • When safeword is disabled, wearer should not be able to change safeword
    ========================================================================
    Owner Trinity runs command:
    /1 kr safeword on
    [13:42] Boston: Safeword is now set to 'on'.
    Kristina sees the following in Nearby Chat
    [13:42] Boston: Safeword is now set to 'on'.
    Kristina runs the following command on her collar
    /kr safeword sendhelp
    [13:47] Boston: Safeword is now set to 'sendhelp'.
    /1 kr safeword
    [13:47] Boston: Safeword is now set to 'sendhelp'.

Owner Trinity runs command:
/1 kr safeword off
[13:50] Boston: Safeword Disabled.
Kristina sees the following in Nearby Chat
[13:50] Boston: Safeword Disabled.
Kristina runs the following command on her collar
/1 kr safeword ImAmDying
[13:52] Boston: Only Owners can set a safeword when disabled!
/1 kr safeword off
[13:53] OpenCollar Stock Collar Beta Testing 2: Only an owner can disable Safeword!

Special Note:
There is a mismatch with the devise name of the collar "[13:38] Boston: Only Owners can set a safeword when disabled!" versus "Boston"
This test passed but the naming mismatch should be fixed

SUCCESSFUL

========================================================================

  • When safeword is disabled and owner sets a new safeword, it should re-enable safeword
    ========================================================================
    Owner Trinity runs commands:
    /1 kr safeword on
    [13:59] Boston: Safeword is now set to 'on'.
    /1 kr safeword off
    [13:59] Boston: Safeword Disabled.
    /1 kr safeword HarvardYard
    [13:59] Boston: Safeword is now set to 'HarvardYard'.

Collar wearer Kristina runs commands:
/1 HarvardYard
[14:02] Boston: You used the safeword, your owners have been notified
[14:02] Boston: RLV was cleared and you may now stand up
[14:02] Boston: Relay temporarily suppressed for 30 seconds due to safeword or clear all.
[14:03] Boston: Relay settings have been restored.
[14:03] Boston: Relay temporarily suppressed for 30 seconds due to safeword or clear all.
[14:03] Boston: Relay settings have been restored.
SUCCESSFUL

@Medea-Destiny
Copy link
Collaborator Author

Medea-Destiny commented Jul 24, 2024

fixes for observations from KristinaHearts' testing

  1. "Only an owner can disable Safeword!" sent via notify rather than llOwnerSay to respect device name.
  2. Checking safeword with "/1 (prefix) safeword" now checks if safeword is disabled and gives clear message informing of that.
  3. Notification for device and safeword changes will always go to both setter and wearer.

Copy link
Collaborator

@Pingout Pingout left a comment

Choose a reason for hiding this comment

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

This fixes the problem with Safeword, and also fixes the problem with Name.
When the collar owner sets the safeword to "off" the safeword is disabled. When the owner sets a new safeword, for example "Blue" the safeword becomes "Blue" and active.
The naming problem now works as expected. Setting the collar name to "Newname" now returns the message "The wearer's name is now set to Newname (if this is the old name, please type '/1 (prefix) name' to confirm the change went through, we may just have lagged)"

@Pingout Pingout removed the Needs testing This issue needs volunteers to try to duplicate the error or identify a caues label Jul 28, 2024
@Medea-Destiny Medea-Destiny added Ready to merge and removed Do Not Merge (yet) One reason or another for not merging labels Jul 28, 2024
@SilkieSabra SilkieSabra merged commit 5e759be into 8.3_Features-branch Jul 28, 2024
2 checks passed
@SilkieSabra SilkieSabra deleted the Medea-Destiny-patch-3 branch July 28, 2024 23:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.3 Target version 8.,3 Ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants