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 the blacklist for unrequire #5

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

skyem123
Copy link

When testing the unrequire tool on OpenOS 1.6.1, I found that it could unload things that broke OpenOS, so I used a process of elimination to find out what not to unload. It seems to work.

Since this involves (removing) the guts of OpenOS (while it's running), maybe @payonel (hopefully this isn't rude) could give his opinion on what is likely to cause issues?

What was odd was that if you unload both term and keyboard, key combinations stopped working, but if you only unloaded one of them it still worked (until you unloaded the other).

When testing the unrequire tool on OpenOS 1.6.1, I found that it could unload things that broke OpenOS, so I used a process of elimination to find out what not to unload. It seems to work.
I ran into this breaking things, kinda surprising considering that you don't need to require it.
Anyway, since there's no way for OpenOS to find it again (even though it still exists), it's on the blacklist.
There won't be much benefit in requiring these anyway, they are built into Lua and so won't be re loadable from file and won't save any memory.
@payonel
Copy link
Contributor

payonel commented May 10, 2017

There is a keyboard service with key event listeners that updates some keyboard state of pressed keys needed for key combo stuff. This keyboard service holds a ref to the original keyboard library, thus if the keyboard library is unloaded the service will still be updating the old lib. And code using the new lib would not be getting key state updates. This is probably why you have to unload term and keyboard together to see kb state failures. It would probably be a bit nicer to NOT blacklist term from unrequire, but instead blacklist keyboard only.

unicode, computer, component, table, string, and math are not provided in /lib but are part of bootstrap
though it is strange you have to unrequire blacklist table, string, and math -- as those are in _G and do not require require

Also removed term following @payonel's suggestion.

(also this commit is a amended commit because I messed up last time)
@skyem123
Copy link
Author

Well... apparently in Lua, you can require things like table, string and math and a whole lot of other stuff that's already in the default environment. So I've added what I could think of.

Should fix the problem with key combinations not working. >_<
@skyem123
Copy link
Author

Oops... I misread, corrected now.

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