search

PC-Logix / OpenSecurity

Security addon for OpenComputers
MIT License
46 stars 24 forks source link

[Bug] Alarmsound does not change if changed right after deactivating it #160

Open GHXX opened 3 years ago

GHXX commented 3 years ago

Describe the bug When calling the .setAlarm() lua function of the alarm block right after calling .deactivate(), and activating it immediately after, then it does not change.

In which environment did the Bug appear? Singleplayer, fresh world.

To Reproduce Steps to reproduce the behavior:

  1. Make a new world
  2. use the /oc_sc command to plop down a opencomputers computer
  3. place an alarm block next to the computer
  4. turn on the pc
  5. open the lua prompt in oc by typing "lua" and hitting enter 6.1. set the variable "a" to be an alarm (a = require("component").sc_alarm) 6.2 paste in the following code snippet (as a single line! you can paste it with middle mouse button): for i=1,#a.listSounds() do al = a.listSounds()[i]:gsub(".ogg",""); print(al);a.setAlarm((al)); a.activate(); os.sleep(5);a.deactivate() end
  6. hit enter
  7. how the first sound thats installed (klaxon2) will play and keep looping, but the other will never play. (i installed 6 additional sounds, but it shouldnt matter here)
  8. now, run this snippet: for i=1,#a.listSounds() do al = a.listSounds()[i]:gsub(".ogg",""); print(al);a.setAlarm((al)); a.activate(); os.sleep(5);a.deactivate();os.sleep(0.5) end and now it will work properly.

Expected behavior The sound should change when deactivating the alarm, changing it an reactivating it. Adding an extra "os.sleep" should not be necessary under any circumstances.

Screenshots / Code Snippet image

Minecraft:

Additional context There are no outputs in the log file while this problem happens.

Ocawesome101 commented 3 years ago

DISCLAIMER: I could be way off here, this an educated guess based on general knowledge of OpenComputers and Minecraft, having largely never looked at the source code of either. I'm not interested in arguing about this; this is just my $0.02 :slightly_smiling_face:

Because of how OpenComputers works, and because Minecraft is limited to 20 ticks per second, and because of concurrency issues (ever gotten a ConcurrentModificationException?) or something like that, certain external (usually world-affecting) values (for example, when you update a redstone wire) updated by the Lua state can only be changed when said Lua state yields, e.g. when os.sleep is called.

Basically, you're not giving the sound time to update.

GHXX commented 3 years ago

Yeah, its fairly safe to say that its not updating quickly enough. No exceptions thrown while testing, but there is definitely some synchronization work to be done on opensecurity in my opinon. Or a cheap waittime.

Shuudoushi commented 3 years ago

Yeah, its fairly safe to say that its not updating quickly enough. No exceptions thrown while testing, but there is definitely some synchronization work to be done on opensecurity in my opinon. Or a cheap waittime.

Um... I don't think you understand what was said... This is most likely a tick rate, or timing, issue. It's not really something that can be fixed by the mod dev. Opencomputers itself has hard baked timing limits to prevent people from crippling servers, last I checked it was something like 2 ticks or so.

CaitlynMainer commented 3 years ago

You've found the one and only fix here. Sleep for a bit after changing the sound. 0.01 is enough to make this work. for i=1,#a.listSounds() do al = a.listSounds()[i]:gsub(".ogg",""); print(al);a.setAlarm((al)); os.sleep(0.01); a.activate(); os.sleep(5);a.deactivate() end Is enough to fix this.

This is 100% a delay in how long it takes OpenComputers to push this change to the TE, and for MC to Sync the TE from the server to the client. ALL OC operations happen on the server TE, and all sound operations happen on the Client TE.

https://github.com/PC-Logix/OpenSecurity/blob/1.12.2/src/main/java/pcl/opensecurity/common/tileentity/TileEntityAlarm.java#L41-L56 which calls https://github.com/PC-Logix/OpenSecurity/blob/1.12.2/src/main/java/pcl/opensecurity/common/tileentity/TileEntityOSSound.java#L86-L89