Skip to content

Conversation

@0x617374726F
Copy link
Contributor

Working on #8 I realized, that we could refactor the scripts iteration. This PR allows us to remove 385 lines of code. Also, this way it will be easier to make necessary adjustments to the iteration once needed.

This could be the case very soon, because I noticed that I was implementing timers (inside my gamemode) in a bad way. Instead of using events::on_tick(), I spawned a new thread that would tick on its own. Therefore I believe, that the panic was intended and the borrow checkers needs to be brought back. I will test it this week and upstream necessary changes.

@0x617374726F
Copy link
Contributor Author

0x617374726F commented May 21, 2025

So I was playing around and found out that my faulty timer implementation was not the issue for the multiple mutable borrows. You can reproduce it by implementing any events inside your gamemode that:

  • are returning values to this crate and
  • trigger new events themselves before they return a value

This is because the mutable reference to the script is being held while waiting for the return value of the event implementation. During the execution, the gamemode triggers a new event that also attempts to borrow the script mutably.

Therefore, I think that the measures taken in #8 must be reverted since it introduces undefined behavior.

@0x617374726F
Copy link
Contributor Author

a4fd118 addresses the double mutable borrow issue. We just need to defer every API call that can potentially create new events that will iterate over the omprs modules. Many thanks to @xxshady for helping me out!

@0x617374726F 0x617374726F changed the title Centralize scripts iteration Get rid of undefined behaviour May 30, 2025
Copy link
Owner

@Sreyas-Sreelal Sreyas-Sreelal left a comment

Choose a reason for hiding this comment

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

Sorry for late reply. Thanks a lot, this is incredible work!

@Sreyas-Sreelal Sreyas-Sreelal merged commit 5feab39 into Sreyas-Sreelal:capi Jun 13, 2025
0 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants