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

A problem in oc_anim **OpenSim issue** #1030

Open
Pingout opened this issue Feb 8, 2024 · 10 comments
Open

A problem in oc_anim **OpenSim issue** #1030

Pingout opened this issue Feb 8, 2024 · 10 comments
Labels
8.3 Target version 8.,3 Bug PRIORITY 1

Comments

@Pingout
Copy link
Collaborator

Pingout commented Feb 8, 2024

What version of OpenCollar are you using? OpenCollar 8.2.3 specifically the oc_anim.lsl script

What behavior did you expect? We have been attempting to port the 8.2.3 version into the OpenSim Alternate Metaverse grid. Almost all of the scripts are able to be used as-is, with the exception of the Collar Animations. It appears that the script correctly hides any poses beginning with the "~" symbol when generating the Animations Menu list. However, the script fails to prevent these hidden poses from being called from the menu. When attempting to play the Beautystand pose, for example, the script will play the ~crawl animation, and so on through the list. This is not a random replacement, the script has placed all hidden poses ahead of the unhidden.

This problem does not appear prior to version 8.x oc_anim script. The oc_anim from version 7.5 works correctly, but does not have the Addons support necessary for Collar Poses to work with the oc_cuff system.

It's possible that the error stems from the fact that in the AMV grid, the poses with special characters are listed ahead of those filenames with standard characters. In Secondlife, the filenames with special characters are listed after the standard filenames. The Upgrade to 8.x changed the oc_anim script so that it uses a temporary variable (Line 144 list lTmp; ) to generate the Animations menu, but then uses the variable g_sPose to actually play the animation. The g_sPose variable is never filtered to remove the special character filenames.

I believe fixing the oc_anim script to work in AMV can be done without any detriment to the script performance in SL

@SilkieSabra SilkieSabra added Bug PRIORITY 1 8.3 Target version 8.,3 labels Feb 8, 2024
@SilkieSabra
Copy link
Contributor

SilkieSabra commented Feb 8, 2024

It looks to me like this is a bug. It's a silent bug in SL because poses with ~ in front of the name are ranked last in the SL grid, but it is an inconsistency in how those are handled in the script, if they are handled in two different ways when generating vs. calling the menu.
All the couples poses start with ~, does this error involve them? and why is there still a ~crawl animation in the collar in AMV? that should not be there.

@Medea-Destiny
Copy link
Collaborator

See lines 201-203. Pose names are stored in lower case, and the command cast to lower case before matching. This is to make the chat commands case-insensitive. The script finds the index of the pose name in the pose list, but then uses that index number with llGetInventoryName. This will be correct if rejected poses starting with ~ have a higher index number than those that start with another character, but not if they don't.

To fix this, as a grid-specific solution that would break in SL, the number of anims starting with ~ could be counted in the getPoseList function (line 146) and stored in a global value. That value then gets added to index in line 202. Though this does require an OpenSim specific version of this script, that's the solution I'd recommend at this point.

A global solution that would work in both SL and grids that order animation names differently would best be done by storing both the original AND lower case versions of each pose. This will double the memory footprint of the pose list so a check on how much memory that script uses should be taken first. A lower script-memory solution would be to loop through animations to compare an llToLower version of anim name to llToLower of the command, however this isn't a good solution. Because the chat commands for the pose menu don't take a command (i.e. /1 [prefix] pose [posename]) but rather work on the pose name alone, all arbitrary commands the collar deals with are parsed by this function. This is already bad, but it's just a quick list check. Looping through and string comparing all animations to every command the collar receives would be pretty horrible.

@Pingout
Copy link
Collaborator Author

Pingout commented Feb 9, 2024

I followed your logic @Medea-Destiny and tested the script using a simple change:
g_sPose = llGetInventoryName(INVENTORY_ANIMATION,index+8);
There are 8 hidden poses in the collar, after having removed the ~crawl animation as suggested by @SilkieSabra
This simple change made all of the poses work. I have not tested the couples poses but i think we can assume they are also now working correctly.
This is still just a temporary fix, as a way to verify Medea's fix.
I still would like to understand, for my own curiosity, why does the 7.x oc_anim work correctly and not the 8.x
What has changed in the version 8.x that makes this patch necessary?
Thank you in advance.

@Medea-Destiny
Copy link
Collaborator

@Pingout I think you'll find that in the OC7 version, the section of code dealing with playing the animation would simply check the named animation is in the pose list and if so play the animation based on the given text. This would make it case sensitive when issuing a pose from chat command.

@Pingout
Copy link
Collaborator Author

Pingout commented Feb 10, 2024

@Pingout I think you'll find that in the OC7 version, the section of code dealing with playing the animation would simply check the named animation is in the pose list and if so play the animation based on the given text. This would make it case sensitive when issuing a pose from chat command.

Thanks Medea! I was thinking the change might have been forced by the Addons, as the collar poses can have chains attached based on a notecard in the right wrist cuff. That would probably necessitate additional communication between the oc_anim and oc_cuff scripts.

@Medea-Destiny
Copy link
Collaborator

The add-on system allows the cuffs to receive internal collar messages. When the anim script plays a pose it'll save the current pose name to settings, which can then be picked up by the cuffs via the add-ons interface and acted on. No change to oc_anim needed.

@amandaleeang
Copy link

amandaleeang commented Apr 29, 2024

Hello,

Some time ago I was trying to get open collar to work in opensim, and I came across that issue.
This is what I implemented, without hard coding the 8 first indices for hidden animations.

Hope it helps

in oc_anim.lsl

/**
Return a list with all the animatons in the inventory without filtering '~xxxx' animations
*/
list GetAllPoseList(integer iType)
{
    // -1 = as it exists in inventory
    // 0 = lower case
    
    list lTmp;
    integer i=0;
    integer end = llGetInventoryNumber(INVENTORY_ANIMATION);
    for(i=0;i<end;i++){
        
        string name = llGetInventoryName(INVENTORY_ANIMATION, i);

            if(iType == -1)lTmp += [name];
            else lTmp += [llToLower(name)];
        
    }
    
    return lTmp;
}

Then, in UserCommand() function

            // This is wrong.
            // integer index = llListFindList(GetPoseList(0), [llToLower(sChangetype)]);
            // g_sPose = llGetInventoryName(INVENTORY_ANIMATION,index);

            // Function GetPoseList() lists will filter those staring with ~, 
            // But Get inventory by index will no filter them
            // so the index does not match the actual index in the inventory

            // Correct one uses a new function GetAllPoseList()
            integer index = llListFindList(GetAllPoseList(0), [llToLower(sChangetype)]);
            g_sPose = llGetInventoryName(INVENTORY_ANIMATION,index);
            
            StartAnimation(g_sPose);
            llMessageLinked(LINK_SET, LM_SETTING_SAVE, "anim_pose="+llList2String(g_lCurrentAnimations, 0),"");
            iRespringPoses=TRUE;
``

@Pingout
Copy link
Collaborator Author

Pingout commented Apr 30, 2024

Hello,

Some time ago I was trying to get open collar to work in opensim, and I came across that issue. This is what I implemented, without hard coding the 8 first indices for hidden animations.

Hope it helps

in oc_anim.lsl

/**
Return a list with all the animatons in the inventory without filtering '~xxxx' animations
*/
list GetAllPoseList(integer iType)
{
    // -1 = as it exists in inventory
    // 0 = lower case
    
    list lTmp;
    integer i=0;
    integer end = llGetInventoryNumber(INVENTORY_ANIMATION);
    for(i=0;i<end;i++){
        
        string name = llGetInventoryName(INVENTORY_ANIMATION, i);

            if(iType == -1)lTmp += [name];
            else lTmp += [llToLower(name)];
        
    }
    
    return lTmp;
}

Then, in UserCommand() function

            // This is wrong.
            // integer index = llListFindList(GetPoseList(0), [llToLower(sChangetype)]);
            // g_sPose = llGetInventoryName(INVENTORY_ANIMATION,index);

            // Function GetPoseList() lists will filter those staring with ~, 
            // But Get inventory by index will no filter them
            // so the index does not match the actual index in the inventory

            // Correct one uses a new function GetAllPoseList()
            integer index = llListFindList(GetAllPoseList(0), [llToLower(sChangetype)]);
            g_sPose = llGetInventoryName(INVENTORY_ANIMATION,index);
            
            StartAnimation(g_sPose);
            llMessageLinked(LINK_SET, LM_SETTING_SAVE, "anim_pose="+llList2String(g_lCurrentAnimations, 0),"");
            iRespringPoses=TRUE;
``

I tried to put this change in to the oc_anim script in OpenSim and it failed. It gives a Stack/Heap error.

I probably did something wrong in my cut and paste. Would it be possible to give us the entire script rather than the two snippets?

Thank you

@Pingout
Copy link
Collaborator Author

Pingout commented May 1, 2024

I was able to get the script working by changing every reference from GetPoseList to GetAllPoseList.
This script plays the correct Pose from the Pose Menu, however the Pose Menu now contains every Pose in inventory, including those marked with the "~"
I think ideally we would like to have both working Pose Menu and Hidden ~xxxx Poses

@amandaleeang
Copy link

Hi,

I did not change every reference from GetPoseList() to GetAllposeList(). Only where I needed to match the index coming from the menu to the index of the animation in the inventory. So hidden poses do not show in the dialog.

Best,

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 Bug PRIORITY 1
Projects
None yet
Development

No branches or pull requests

4 participants