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

Replace "return R()" with "return" in R() function. #1005

Closed

Conversation

neil2ball
Copy link
Contributor

2023-11-06 00:29:39,250 INFO [YEngine]: - <229.76128, 186.58499, 21.009304> OpenCollar - 8.2.3:oc_folders
2023-11-06 00:29:39,251 INFO [YEngine]: - (157,58) Error: function returns void, no value allowed
2023-11-06 00:29:39,251 INFO [YEngine]: - (158,59) Error: function returns void, no value allowed
2023-11-06 00:29:39,252 INFO [YEngine]: - (159,56) Error: function returns void, no value allowed
2023-11-06 00:29:39,252 INFO [YEngine]: - (160,57) Error: function returns void, no value allowed
2023-11-06 00:29:39,253 INFO [YEngine]: - (161,47) Error: function returns void, no value allowed
2023-11-06 00:29:39,253 INFO [YEngine]: - (209,61) Error: function returns void, no value allowed
2023-11-06 00:29:39,254 INFO [YEngine]: - (210,62) Error: function returns void, no value allowed
2023-11-06 00:29:39,255 INFO [YEngine]: - (211,59) Error: function returns void, no value allowed
2023-11-06 00:29:39,256 INFO [YEngine]: - (212,60) Error: function returns void, no value allowed

LSL is not my area of expertise, but I wouldn't think that you would try to use R() as a recursive function. OpenSim's YEngine does not like this as it views R() as a void.

@NikkiLacrima
Copy link
Contributor

NikkiLacrima commented Nov 6, 2023

The return R() is not in the R() function, it is in the following Browser(key kID, integer iAuth, string sPath) function.

Original code is correct, and it is not a recursive call. The YEngine might be happy with something like :
if(iAuth == CMD_TRUSTED && !Bool((g_iAccessBitSet&1))) { R(); return; }

@neil2ball
Copy link
Contributor Author

Sorry, it was getting late when I was working on this. This must be some kind of error in the YEngine then. I'll forward this to OpenSim.

@Pingout Pingout added More Info Needed Do Not Merge (yet) One reason or another for not merging Out of Scope Things that aren't really OC problems to fix. labels Nov 6, 2023
@Pingout
Copy link
Collaborator

Pingout commented Nov 6, 2023

Is this script change for OpenSIM only? And if so, we can not point it to the Master repo.
Should we start a repository for exclusively OpenSIM script?

@NikkiLacrima
Copy link
Contributor

NikkiLacrima commented Nov 6, 2023

The patch does not change any fuctionality for LSL open collar, and the current return R(); construct is formally not correct even if it works for LSL. The function Browser(key kID, integer iAuth, string sPath) does not have a return value and type so there should not be a "return value;"

I think the PR in the form of the second commit can be approved, after testing, but it should be retargeted to 8.3_Features-branch.

@Pingout Pingout added Needs testing This issue needs volunteers to try to duplicate the error or identify a caues and removed More Info Needed Out of Scope Things that aren't really OC problems to fix. labels Nov 6, 2023
@Pingout Pingout changed the base branch from master to 8.3_Features-branch November 6, 2023 16:41
@NikkiLacrima
Copy link
Contributor

NikkiLacrima commented Nov 6, 2023

Please use 4 spaces and not tabs, one never knows how many space a tab will be turned into. Especially when moving around between SL script editor, GitHub webb and other script editors for comparison checks.

@neil2ball
Copy link
Contributor Author

Please use 4 spaces and not tabs, one never knows how many space a tab will be turned into. Especially when moving around between SL script editor, GitHub webb and other script editors for comparison checks.

I'm not quite sure why it changed the spacing. I just used LSL Editor, but I have a feeling that isn't what people use these days. I'll make another edit to the file to include four spaces instead.

@Medea-Destiny
Copy link
Collaborator

Agree with @NikkiLacrima that the return R(); though it functions is not good practice and can simply be replaced by {R(); return;}. While not necessary for SL let's do it.

However the formatting here is problematic with a lot of change to spacing that misaligns the top comments badly once inserted into an SL script, and creates a +573/-573 line change that may cause confusion in future. Can we have an edit of the file as it currently exists in the OpenCollarTeam:8.3_Features-branch https://github.com/OpenCollarTeam/OpenCollar/blob/8.3_Features-branch/src/collar/oc_folders.lsl with just the return R(); statements replaced?

@Medea-Destiny
Copy link
Collaborator

Looks good, thanks @neil2ball

@NikkiLacrima
Copy link
Contributor

Tested in world, all seems good. Approved

@neil2ball
Copy link
Contributor Author

Sorry if this was a mistake. I'm a button presser.

@NikkiLacrima
Copy link
Contributor

NikkiLacrima commented Nov 13, 2023

I dont know how this can affect the OpenCollar/8.3_Features-branch, one possible way to handle is to close/retire this PR and I create a new clean branch and PR with the agreed changes, this one has become a jumble of commits that are then overwriting each others.

@neil2ball neil2ball closed this Nov 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Do Not Merge (yet) One reason or another for not merging Needs testing This issue needs volunteers to try to duplicate the error or identify a caues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants