Magisk-Modules-Repo / submission

Submit your Magisk Modules!
567 stars 89 forks source link

[Submission] OnePlus 3/3T Front Camera Fix #414

Closed MarcAnt01 closed 5 years ago

MarcAnt01 commented 5 years ago

https://github.com/MarcAnt01/OnePlus-3-3T-Front-Camera-Fix

Didgeridoohan commented 5 years ago

You're placing a script in service.d, but you don't have any uninstall logic in place, so that script will still run when the module is uninstalled. Use the module service.sh instead, same thing.

And, you should put your compatibility checks as early as possible, not last in the installation process...

MarcAnt01 commented 5 years ago

It uses the same logic as slowmofix module in the repo, service.sh doesn't work properly on some Roms, what would you advice?

Didgeridoohan commented 5 years ago

What do you mean that it doesn't work properly? That's the absolute first I hear about it and I use service.sh quite heavily in MagiskHide Props Config.

MarcAnt01 commented 5 years ago

Yes and seems to happen just with Installer and not with the old template. This issue is limited just to some Custom Roms, with a lot of features, so I suppose they cause some problems to late service script.

Didgeridoohan commented 5 years ago

That doesn't make any sense... Could you provide more a more detailed description of this issue?

For acceptance to the repo using the service.sh script is preferable, but in a pinch, providing a proper uninstall logic for the script could also be acceptable. This should also be fixed in slowmofix...

MarcAnt01 commented 5 years ago

Sure, am busy today, tomorrow will create uninstall.sh logic and fix the slowmo module, as well

MarcAnt01 commented 5 years ago

Done @Didgeridoohan

Didgeridoohan commented 5 years ago

Ok... The script will now be uninstalled together with the module. But, it will still be installed even if an incompatible device is detected. As I stated previously, you need to move your compatibility check earlier.

MarcAnt01 commented 5 years ago

Ok... The script will now be uninstalled together with the module. But, it will still be installed even if an incompatible device is detected. As I stated previously, you need to move your compatibility check earlier.

Sure, I had already fixed, but forgot to push, here you go

MagiskBot commented 5 years ago

New repo added: op3frontcamfix Please accept the collaboration invitation in your email.

MarcAnt01 commented 5 years ago

OK, thanks for the review and the help😁