search

PhoenicisOrg / scripts

Phoenicis scripts
GNU Lesser General Public License v3.0
64 stars 49 forks source link

Change Plugins to use the Module System #1131

Closed madoar closed 4 years ago

madoar commented 4 years ago

This PR changes the plugins to use the module system.

madoar commented 4 years ago

@ImperatorS79 @Zemogiter can you please check if the changes are working as intended @plata if it is ok I would like to add a documentation for the plugins in a separate issue. As far as I remember we have no documentation for plugins yet so it is takes more effort compared to simply fixing an existing documentation :)

Travis fails because it can't build the github pages @plata any ideas?

plata commented 4 years ago

I'm fine to update the documentation separately. Regarding the GitHub pages: https://community.netlify.com/t/jekyll-builds-consistently-failing-with-error-superclass-must-be-a-class-faraday-deprecatedconstant-given/3799/7.

madoar commented 4 years ago

It seems like a restart of travis fixed the issue :) Any change requests?

plata commented 4 years ago

There was another update to faraday. Probably that fixed it.

Zemogiter commented 4 years ago

I think the old way we defined the dll override in Anno 2070 scripts was more readable.

madoar commented 4 years ago

@Zemogiter I agree the previous way how we accessed verbs and plugins was more readable. The issue was just that it was not as safe to use as it is now. For example different plugins and verbs overrode the same functions of the Wine class. This can lead to very unexpected behavior. In addition we had problems with missing includes or unused includes. The new verb and plugin implementations fix these issues.

Maybe it is important to note that we can still improve the implementations. While we know that they are not perfect we may find solutions to the identified issues and improve on them later!