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

CheckPBOs in ACE_Settings #1236

Merged
merged 4 commits into from Jun 4, 2015
Merged

CheckPBOs in ACE_Settings #1236

merged 4 commits into from Jun 4, 2015

Conversation

jonpas
Copy link
Member

@jonpas jonpas commented May 15, 2015

@jonpas jonpas changed the title Added CheckPBOs to ACE_Settings CheckPBOs in ACE_Settings May 15, 2015
@jaynus jaynus added the kind/enhancement Release Notes: **IMPROVED:** label May 15, 2015
@jaynus jaynus added this to the 3.1.0 milestone May 15, 2015
@thojkooi
Copy link
Contributor

This looks good. I will look at it in more detail later today.

@nicolasbadano
Copy link
Contributor

If the settings command to do so, the pbo checking code should be run after postInit without needing the module.

@jonpas
Copy link
Member Author

jonpas commented May 17, 2015

I think that requires some restructuring if I understood what you mean (running check version without needed the module placed)?

@nicolasbadano
Copy link
Contributor

Yes. I think it's simply a matter of installing a "SettingsInitialized" even handler that runs the module code if the correct settings are set. That routine and the module should set some flag to avoid checking it twice if the module is present.

@ViperMaul
Copy link
Contributor

@jonpas, @Glowbal
I would really like to see this one completed for 3.1.0 in the next 3 days if possible.

@jonpas
Copy link
Member Author

jonpas commented May 26, 2015

I will have time to work on it tomorrow or Thursday. I don't particularly get @esteldunedain 's comment though, I have to dive into the code way more. 😁

@jonpas
Copy link
Member Author

jonpas commented May 27, 2015

@esteldunedain I am actually confused by your statement, SettingsInitialized hasn't been added yet as far as I can see, so what exactly do you want me to do here?

@nicolasbadano
Copy link
Contributor

@esteldunedain I am actually confused by your statement, SettingsInitialized hasn't been added yet as far as I can see, so what exactly do you want me to do here?

It was merged today, so it should be gtg

@jonpas
Copy link
Member Author

jonpas commented May 27, 2015

Yep yep, will work on it today or tomorrow. Thanks!

@jonpas
Copy link
Member Author

jonpas commented May 28, 2015

I did what @esteldunedain wanted or at least I think I did. I streamlined module settings loading and then calling the actual function from withing SettingsInitialized, it all works locally it seems but it should probably be tested on dedicated as well, if someone can do that it would be greatly appreciated.

@thojkooi
Copy link
Contributor

This is now always checking pbos, no?

Script looks ok from that I can tell, just need someone to test this on a dedicated server with JIP, like @jonpas said.

@jonpas
Copy link
Member Author

jonpas commented May 28, 2015

AFAIK it will now always check ACE3 PBOs but only display warnings, if checkPBOsCheckAll is set to true then it will check all.

Is it by default that when no module is present it doesn't check at all? TBH I'd prefer it always checks, it gets rid of a lot of wrong testing setups where players have different ACE3 versions installed.

Code itself has to be cleaned up at some point as well though.

@thojkooi thojkooi merged commit baca5a2 into acemod:master Jun 4, 2015
@thojkooi
Copy link
Contributor

thojkooi commented Jun 4, 2015

Thank you @jonpas

@jonpas jonpas deleted the checkpboSettings branch June 4, 2015 20:31
@jonpas
Copy link
Member Author

jonpas commented Jun 4, 2015

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Release Notes: **IMPROVED:** status/needs-testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants