Skip to content

Add optional param to specify scripting directory#2

Closed
leojonathanoh wants to merge 7 commits intostartersclan:masterfrom
leojonathanoh:add-optional-parameter-to-specify-scripting-directory
Closed

Add optional param to specify scripting directory#2
leojonathanoh wants to merge 7 commits intostartersclan:masterfrom
leojonathanoh:add-optional-parameter-to-specify-scripting-directory

Conversation

@leojonathanoh
Copy link
Copy Markdown
Member

Implements #1

@leojonathanoh leojonathanoh force-pushed the add-optional-parameter-to-specify-scripting-directory branch from ae6cfd0 to 9c40215 Compare April 27, 2020 01:09
@leojonathanoh leojonathanoh self-assigned this Apr 27, 2020
@leojonathanoh
Copy link
Copy Markdown
Member Author

there are some false positives. Need to fix tests first

Comment thread src/Compile-SourceScript/Public/Compile-SourceScript.ps1 Outdated
@leojonathanoh leojonathanoh force-pushed the add-optional-parameter-to-specify-scripting-directory branch from 6769c35 to f43195a Compare April 27, 2020 04:17
@leojonathanoh
Copy link
Copy Markdown
Member Author

warning, above green checks are false positives (false test passes)

@joeltimothyoh
Copy link
Copy Markdown
Member

joeltimothyoh commented Apr 27, 2020

Actually, I need clarification on why the scripting directory should require specification. It is mandatory to specify the path to the script file via the -File parameter. The variable $sourceFile whose property DirectoryName is what is used to determine the scripting directory; in other words, the path to the scripting directory is simply the full path of the directory containing the specified plugin. Specifying the scripting directory would so be redundant in any case.

@leojonathanoh
Copy link
Copy Markdown
Member Author

leojonathanoh commented Apr 27, 2020

Actually, I need clarification on why the scripting directory should require specification. It is mandatory to specify the path to the script file via the -File parameter. The variable $sourceFile whose property DirectoryName is what is used to determine the scripting directory; in other words, the path to the scripting directory is simply the full path of the directory containing the specified plugin. Specifying the scripting directory would so be redundant in any case.

Specifying the scripting directory allows the separation of a plugin source file .sma and .sp from the compiler and include files (scripting directory)

@joeltimothyoh
Copy link
Copy Markdown
Member

joeltimothyoh commented Apr 27, 2020

Specifying the scripting directory allows the separation of a plugin source file .sma and .sp from the compiler and include files (scripting directory)

Correct me if I'm wrong, but say a plugin's script file by the name plugin1.sp lives within a scripting directory called scripts instead. Say the full path to the scripting directory is /home/username/scripts. To compile the sourcemod plugin on linux, the command Compile-SourceScript -File /home/username/scripts/plugin.sp -Force -SkipWrapper simply needs to be issued. The scripting directory would be determined to be /home/username/scripts based on the specified plugin's determined full path alone. There is therefore no need for specifying the scripting directory.

@leojonathanoh
Copy link
Copy Markdown
Member Author

Specifying the scripting directory allows the separation of a plugin source file .sma and .sp from the compiler and include files (scripting directory)

Correct me if I'm wrong, but say a plugin's script file by the name plugin1.sp lives within a scripting directory called scripts instead. Say the full path to the scripting directory is /home/username/scripts. To compile the sourcemod plugin on linux, the command Compile-SourceScript -File /home/username/scripts/plugin.sp -Force -SkipWrapper simply needs to be issued. The scripting directory would be determined to be /home/username/scripts based on the specified plugin's determined full path alone. There is therefore no need for specifying the scripting directory.

I'll give u an example. Say i have a plugin in /home/username/myplugin.sp. I also have a scripting directory in /home/username/sourcemod/scripting. Now I can call:

Compile-SourceScript -File /home/username/myplugin.sp -ScriptingDirectory /home/username/sourcemod/scripting -Force -SkipWrapper

where the plugin source lives outside the scripting directory, away from includes and the compiler.

@leojonathanoh leojonathanoh force-pushed the add-optional-parameter-to-specify-scripting-directory branch from 15b7d02 to dfc41e5 Compare May 3, 2020 21:25
@leojonathanoh leojonathanoh dismissed joeltimothyoh’s stale review May 3, 2020 22:32

fixed in later commits

@joeltimothyoh
Copy link
Copy Markdown
Member

joeltimothyoh commented May 3, 2020

I know I worked on this PR. And you happen to be working on it right now. Yet, the more time I spend supporting this feature request, the more it seems an unnecessary and confusing feature to have.

Speaking of the default behavior of Compile-SourceScript notwithstanding this PR, when a plugin source file is specified without -ScriptingDirectory, the scripting directory is automatically determined to be the directory containing the specified file. The default behavior therefore already allows flexibility in how the scripting directory is named, with the exception of relative directories compiled and plugins which will be automatically created and cannot be customized.

The first issue with -ScriptingDirectory is how it actually requires -SkipWrapper to be passed. If the switch isn’t passed, compilation of the specified plugin source file would not occur correctly since the wrapper scripts simply invoke the compiler to compile all plugins in the work directory, in which case would be determined to be the specified scripting directory which would not include compilation of the specified source file. The compiler wrapper script in amxmod compiles all plugin source files existing within the very directory it is in, a convention adopted and also followed by sourcemod‘s own compiler wrapper. Conventions usually exist for a reason, and I think they should be followed closely for the sake of the community who may stumble across this convenient wrapper solution. Yes, Compile-SourceScript is itself a convenient wrapper that is designed to support invocation of the two mods’ compiler wrappers; not the most elegant way for compiling plugins, but at least supported by the module that makes the module predictable to use. Yet, what adding -ScriptingDirectory would do is having to prevent compiler wrappers from being invoked unless the directory containing the specified plugin source file is itself like the default scripting directory - that is, containing the compiler, compiler wrapper, and include files, as well as other plugins source files with their include files as well. As though this isn’t confusing enough, there’s too the relative compiled directory which will be created and contain compiled plugins, as well as the plugin directory for installation of the plugin, both of which would have to exist in the directory containing the specified plugin source file. On could argue to automatically have -SkipWrapper passed when -ScriptingDirectory is specified. Still, supporting so would only add complication to the current parameters offered by Compile-SourceScript in now having to have parameter sets that ensure -SkipWrapper is on when -ScriptingDirectory is passed.

If Compile-SourceScript is going to be easy and predictable to use, then even the parameters themselves should be given names that accurately represent the functionality they serve. As I’ve described earlier, the scripting directory in both amxmodx and sourcemod isn’t simply a directory containing plugin source files, a.k.a. scripts as they would call, but is one containing the compiler and all default include files that come with the mods. With that in mind, a custom scripting directory would actually be one containing those very files themselves. That means, by convention, that the location of the source file cannot actually live outside a scripting directory. Seeing -ScriptingDirectory as separate from the plugin source file and especially allowing specification of the two as separate parameters goes against the convention set by the mods and would very likely come across as confusing to users of this module, as it has been for me.

I think supporting this change requires more thought than it may seem to require. If anything, I think the module shouldn’t be overly restrictive as a compiling solution or make the mistake of becoming confusing to users. Remember that the compilers themselves take in parameters. I think anyone who sufficiently wishes to have their compile process customized (source file path, compiled directory path, installation/copy processes), may as well do so with their own scripts for directly invoking the compiler and not the compile wrapper. It would not surprise me if anyone would much rather do so than use Compile-SourceScript if the module ends up coming across as limiting and unintuitive.

@leojonathanoh
Copy link
Copy Markdown
Member Author

Compile-SourceScript is a better wrapper than the included wrappers:

  • amxmodx's compile.sh for *nix, and compile.exe for Windows
  • sourcemod's compile.sh for *nix, and compile.exe for Windows

Would be better if the default behavior was -Skipwrapper, so that it makes it apparent what problem this module is solving, which is to replace the included wrappers.

@joeltimothyoh
Copy link
Copy Markdown
Member

Compile-SourceScript is a better wrapper than the included wrappers:

* `amxmodx`'s `compile.sh` for *nix, and `compile.exe` for Windows

* `sourcemod`'s `compile.sh` for *nix, and `compile.exe` for Windows

Would be better if the default behavior was -Skipwrapper, so that it makes it apparent what problem this module is solving, which is to replace the included wrappers.

I've indeed thought about having -Skipwrapper be the default behavior. Yet, I think an important element of what the module solves is checking the compiled directory for changes since the compilation was run, as well as installing the plugin into the plugins directory. These work in accordance to the conventions of both amxmodx and sourcemod, which should work well for most users of the module who are likely developers of plugin themselves.

Another important reason for having provided the choice for invoking the compiler or the compiler wrapper is in how the plugin needn't be recompiled if possible. On Windows, sourcemod's compiler wrapper compile.exe prevents the plugin from having to be compiled unnecessarily by recording the last successful compile date in compile.txt that I think is checked against the matching plugin's modified date, but not the compiler. The same is true for Windows's amxmodx's compiler amxxpc.exe, but not the compiler wrapper compile.exe. On Linux, plugins are always re-compiled regardless of changes. So really, the option is simply for ease of development, and should come across as familiar for users of the module, granting the option should they know the difference between the compilers and their wrappers and how compiling behavior differs on depending on the OS.

@leojonathanoh
Copy link
Copy Markdown
Member Author

I've indeed thought about having -Skipwrapper be the default behavior. Yet, I think an important element of what the module solves is checking the compiled directory for changes since the compilation was run, as well as installing the plugin into the plugins directory. These work in accordance to the conventions of both amxmodx and sourcemod, which should work well for most users of the module who are likely developers of plugin themselves.

Yes. It's is a better wrapper.

Another important reason for having provided the choice for invoking the compiler or the compiler wrapper is in how the plugin needn't be recompiled if possible. On Windows, sourcemod's compiler wrapper compile.exe prevents the plugin from having to be compiled unnecessarily by recording the last successful compile date in compile.txt that I think is checked against the matching plugin's modified date, but not the compiler. The same is true for Windows's amxmodx's compiler amxxpc.exe, but not the compiler wrapper compile.exe. On Linux, plugins are always re-compiled regardless of changes. So really, the option is simply for ease of development, and should come across as familiar for users of the module, granting the option should they know the difference between the compilers and their wrappers and how compiling behavior differs on depending on the OS.

To me, tracking whether a source file has changed by modified date is not very reliable. Just do a git checkout on a source file that the "caching" behavior of using compile.txt is gone. I would prefer to assume that each compile should always compile the plugin .

@joeltimothyoh
Copy link
Copy Markdown
Member

To me, tracking whether a source file has changed by modified date is not very reliable. Just do a git checkout on a source file that the "caching" behavior of using compile.txt is gone. I would prefer to assume that each compile should always compile the plugin .

I know it isn't reliable, but it works pretty well especially on Windows, as I've found -SkipWrapper to be useful for compiling sourcemod plugins. Removing the option to invoke the wrapper script will remove the usefulness that comes along with it.

Unless the module transforms into a particularly intelligent wrapper script while maintaining ease of use, I doubt it would match up to users writing a simple script for invoking the compiler with exact precision without the need for PowerShell, Compile-SourceScript, or being limited by the parameters supported by the module.

@leojonathanoh
Copy link
Copy Markdown
Member Author

I know it isn't reliable, but it works pretty well especially on Windows, as I've found -SkipWrapper to be useful for compiling sourcemod plugins. Removing the option to invoke the wrapper script will remove the usefulness that comes along with it.

We could support our own version of compiled.dat (or named something else) which contains a filename and a hash instead.

Unless the module transforms into a particularly intelligent wrapper script while maintaining ease of use, I doubt it would match up to users writing a simple script for invoking the compiler with exact precision without the need for PowerShell, Compile-SourceScript, or being limited by the parameters supported by the module.

yes. So the module should be solving problems for a majority of users, except those power users. It should not be too complicated.

Even hacking up shell scripts will still have to deal juggling between using windows / nix compilers and their wrappers by checking for the platform, are less extensible, less testable, and is not as platform-agnostic as Powershell.

@joeltimothyoh
Copy link
Copy Markdown
Member

We could support our own version of compiled.dat (or named something else) which contains a filename and a hash instead.

This is a terrible idea. Proprietary files should be avoided at all costs.

yes. So the module should be solving problems for a majority of users, except those power users. It should not be too complicated.

So let's keep the module simple and straightforward to use.

@leojonathanoh
Copy link
Copy Markdown
Member Author

This is a terrible idea. Proprietary files should be avoided at all costs.

Agreed. But it would "cache" better than the original wrapper.

So let's keep the module simple and straightforward to use.

To keep it simple, -SkipWrapper should be the default behavior. Wrapper calling wrapper should no longer be supported.

@joeltimothyoh
Copy link
Copy Markdown
Member

This is a terrible idea. Proprietary files should be avoided at all costs.

Agreed. But it would "cache" better than the original wrapper.

So let's keep the module simple and straightforward to use.

To keep it simple, -SkipWrapper should be the default behavior. Wrapper calling wrapper should no longer be supported.

I think you need to think things through instead of dishing out quick replies.

As I've pointed out in #2 (comment), it's a good feature to support invocation of the wrapper script because it does provide some usefulness on Windows.

An important behavior to note is, should the wrapper script be invoked causing all unspecified plugins to be compiled, all successfully compiled plugins will end up in the compiled directory as though users directly executed the default compiler wrapper - the difference lies in how only the specified plugin will be installed in the plugins directory.

@leojonathanoh
Copy link
Copy Markdown
Member Author

leojonathanoh commented May 4, 2020

I think you need to think things through instead of dishing out quick replies.

As I've pointed out in #2 (comment), it's a good feature to support invocation of the wrapper script because it does provide some usefulness on Windows.

On windows one could do

# By pipeline
Get-Item C:/path/to/scripting/*.sp | Compile-SourceScript -SkipWrapper -Force
# By array (not yet supported)
Compile-SourceScript -SourceFile (Get-Item C:/path/to/scripting/*.sp) -SkipWrapper -Force

which compiles all files in the scripting directory.

An important behavior to note is, should the wrapper script be invoked causing all unspecified plugins to be compiled, all successfully compiled plugins will end up in the compiled directory as though users directly executed the default compiler wrapper - the difference lies in how only the specified plugin will be installed in the plugins directory.

Yes, this is the one feature that differentiates this wrapper drastically from the original wrappers compile.sh and compile.exe.

@joeltimothyoh
Copy link
Copy Markdown
Member

# By pipeline
Get-Item C:/path/to/scripting/*.sp | Compile-SourceScript -SkipWrapper -Force
# By array (not yet supported)
Compile-SourceScript -SourceFile (Get-Item C:/path/to/scripting/*.sp) -SkipWrapper -Force

which compiles all files in the scripting directory.

Yes, we can support piping and specification of multiple plugin source files in future.

An important behavior to note is, should the wrapper script be invoked causing all unspecified plugins to be compiled, all successfully compiled plugins will end up in the compiled directory as though users directly executed the default compiler wrapper - the difference lies in how only the specified plugin will be installed in the plugins directory.

Yes, this is the one feature that differentiates this wrapper drastically from the original wrappers compile.sh and compile.exe.

The module also checks for differing hashes right before and after compilation. Due to the statefulness of the compiled directory, I felt it was important to perform the check especially in the case where -Force isn't specified, wherein some plugins may have changed from the time the user compiles the plugin and before the decision to install the plugin is made, and how the user will be alerted of all other plugins that have changed within the compiled directory since the beginning of the compilation process.

@leojonathanoh
Copy link
Copy Markdown
Member Author

Yes, we can support piping and specification of multiple plugin source files in future.

Yes, it is to be expected, much like Get-Item and Get-ChildItem which allows [string] and [array] objects passed to -File.

The module also checks for differing hashes right before and after compilation. Due to the statefulness of the compiled directory, I felt it was important to perform the check especially in the case where -Force isn't specified, wherein some plugins may have changed from the time the user compiles the plugin and before the decision to install the plugin is made, and how the user will be alerted of all other plugins that have changed within the compiled directory since the beginning of the compilation process.

Yes, this interactive feature of being able to decide whether to compile and install or simply compile is another bonus of this wrapper. Would be good if the readme listed exactly how this module helps developer workflow.

@joeltimothyoh
Copy link
Copy Markdown
Member

Yes, we can support piping and specification of multiple plugin source files in future.

Yes, it is to be expected, much like Get-Item and Get-ChildItem which allows [string] and [array] objects passed to -File.

I'll say it's a neat feature to have, though not exactly useful since users would usually be compiling one plugin at a time.

The module also checks for differing hashes right before and after compilation. Due to the statefulness of the compiled directory, I felt it was important to perform the check especially in the case where -Force isn't specified, wherein some plugins may have changed from the time the user compiles the plugin and before the decision to install the plugin is made, and how the user will be alerted of all other plugins that have changed within the compiled directory since the beginning of the compilation process.

Yes, this interactive feature of being able to decide whether to compile and install or simply compile is another bonus of this wrapper. Would be good if the readme listed exactly how this module helps developer workflow.

Care to contribute what you can to the repo?

@leojonathanoh
Copy link
Copy Markdown
Member Author

Yes, we can support piping and specification of multiple plugin source files in future.

Yes, it is to be expected, much like Get-Item and Get-ChildItem which allows [string] and [array] objects passed to -File.

I'll say it's a neat feature to have, though not exactly useful since users would usually be compiling one plugin at a time.

The module also checks for differing hashes right before and after compilation. Due to the statefulness of the compiled directory, I felt it was important to perform the check especially in the case where -Force isn't specified, wherein some plugins may have changed from the time the user compiles the plugin and before the decision to install the plugin is made, and how the user will be alerted of all other plugins that have changed within the compiled directory since the beginning of the compilation process.

Yes, this interactive feature of being able to decide whether to compile and install or simply compile is another bonus of this wrapper. Would be good if the readme listed exactly how this module helps developer workflow.

Care to contribute what you can to the repo?

I can contribute both PRs, but I fear merge conflicts, so i prefer to solve this PR as well as #7 first

@joeltimothyoh
Copy link
Copy Markdown
Member

I can contribute both PRs, but I fear merge conflicts, so i prefer to solve this PR as well as #7 first

I think by now you already should know my stance on this PR.

@leojonathanoh
Copy link
Copy Markdown
Member Author

leojonathanoh commented May 4, 2020

I think by now you already should know my stance on this PR.

Not exactly clear. As you've mentioned further up in #2 (comment), adding ScriptingDirectory will require -SkipWrapper to be passed. So it's all about whether we decide to make -SkipWrapper the default, and remove wrapper support altogether.

@joeltimothyoh
Copy link
Copy Markdown
Member

I think by now you already should know my stance on this PR.

Not exactly clear. As you've mentioned further up in #2 (comment), adding ScriptingDirectory will require -SkipWrapper to be passed. So it's all about whether we decide to make -SkipWrapper the default, and remove wrapper support altogether.

You may wish to go over the thread carefully starting from #2 (comment).

@leojonathanoh
Copy link
Copy Markdown
Member Author

leojonathanoh commented May 4, 2020

You may wish to go over the thread carefully starting from #2 (comment).

Let me break my responses to #2 (comment) down.

I know I worked on this PR. And you happen to be working on it right now. Yet, the more time I spend supporting this feature request, the more it seems an unnecessary and confusing feature to have.

Speaking of the default behavior of Compile-SourceScript notwithstanding this PR, when a plugin source file is specified without -ScriptingDirectory, the scripting directory is automatically determined to be the directory containing the specified file. The default behavior therefore already allows flexibility in how the scripting directory is named, with the exception of relative directories compiled and plugins which will be automatically created and cannot be customized.

The first issue with -ScriptingDirectory is how it actually requires -SkipWrapper to be passed. If the switch isn’t passed, compilation of the specified plugin source file would not occur correctly since the wrapper scripts simply invoke the compiler to compile all plugins in the work directory, in which case would be determined to be the specified scripting directory which would not include compilation of the specified source file. The compiler wrapper script in amxmod compiles all plugin source files existing within the very directory it is in, a convention adopted and also followed by sourcemod‘s own compiler wrapper. Conventions usually exist for a reason, and I think they should be followed closely for the sake of the community who may stumble across this convenient wrapper solution. Yes, Compile-SourceScript is itself a convenient wrapper that is designed to support invocation of the two mods’ compiler wrappers; not the most elegant way for compiling plugins, but at least supported by the module that makes the module predictable to use. Yet, what adding -ScriptingDirectory would do is having to prevent compiler wrappers from being invoked unless the directory containing the specified plugin source file is itself like the default scripting directory - that is, containing the compiler, compiler wrapper, and include files, as well as other plugins source files with their include files as well. As though this isn’t confusing enough, there’s too the relative compiled directory which will be created and contain compiled plugins, as well as the plugin directory for installation of the plugin, both of which would have to exist in the directory containing the specified plugin source file. On could argue to automatically have -SkipWrapper passed when -ScriptingDirectory is specified. Still, supporting so would only add complication to the current parameters offered by Compile-SourceScript in now having to have parameter sets that ensure -SkipWrapper is on when -ScriptingDirectory is passed.

My opinion is simple, as you mentioned:, "Compile-SourceScript is itself a convenient wrapper that is designed to support invocation of the two mods’ compiler wrappers". I agree with the first part that it is a convenient wrapper. But it is more than that, it is a better wrapper (argued in #2 (comment) and #2 (comment)) and should replace both wrappers compile.exe and compile.sh. And if it is actually a full replacement, -SkipWrapper will be the default behavior and the -SkipWrapper param can be removed.

Now -ScriptingDirectory comes into the picture, since it only makes sense when the actual compiler is called. Thats why i said, whether we implement the -ScriptingDirectory param depends on whether we decide to remove support for wrappers compile.exe and compile.sh.

If Compile-SourceScript is going to be easy and predictable to use, then even the parameters themselves should be given names that accurately represent the functionality they serve. As I’ve described earlier, the scripting directory in both amxmodx and sourcemod isn’t simply a directory containing plugin source files, a.k.a. scripts as they would call, but is one containing the compiler and all default include files that come with the mods. With that in mind, a custom scripting directory would actually be one containing those very files themselves. That means, by convention, that the location of the source file cannot actually live outside a scripting directory. Seeing -ScriptingDirectory as separate from the plugin source file and especially allowing specification of the two as separate parameters goes against the convention set by the mods and would very likely come across as confusing to users of this module, as it has been for me.

Following the same line of thought, if Compile-SourceScript is indeed a replacement wrapper, there is one more argument for it being a better wrapper than the native wrappers compile.exe and compile.sh: Specifying -ScriptingDirectory is that it allows better developer workflow by separating plugin source code from the compiler. Now the developer can 1) choose a desired compiler 2) Keep plugin source code it its own repo, away from the compiler.

I think supporting this change requires more thought than it may seem to require. If anything, I think the module shouldn’t be overly restrictive as a compiling solution or make the mistake of becoming confusing to users. Remember that the compilers themselves take in parameters. I think anyone who sufficiently wishes to have their compile process customized (source file path, compiled directory path, installation/copy processes), may as well do so with their own scripts for directly invoking the compiler and not the compile wrapper. It would not surprise me if anyone would much rather do so than use Compile-SourceScript if the module ends up coming across as limiting and unintuitive.

The most intuitive way to use this module is:

Get-Item C:/path/to/my.sp | Compile-SourceScript -Force

where, as i argued earlier, there is no support for native wrappers, and -SkipWrapper is the default behavior and removed.


You may wish to go over the thread carefully starting from #2 (comment).

So if you followed my arguments, as I mentioned in #2 (comment), whether this PR is implemented depends on whether -SkipWrapper is made default behavior and wrapper support is removed altogether, so where the developer intuitively knows how to use the module:

Get-Item C:/path/to/my.sp | Compile-SourceScript -Force -ScriptingDirectory C:/path/to/some/compiler/scripting

@joeltimothyoh
Copy link
Copy Markdown
Member

joeltimothyoh commented May 4, 2020

Yes, Compile-SourceScript is itself a convenient wrapper that is designed to support invocation of the two mods’ compiler wrappers; not the most elegant way for compiling plugins, but at least supported by the module that makes the module predictable to use. Yet, what adding -ScriptingDirectory would do is having to prevent compiler wrappers from being invoked unless the directory containing the specified plugin source file is itself like the default scripting directory - that is, containing the compiler, compiler wrapper, and include files, as well as other plugins source files with their include files as well. As though this isn’t confusing enough, there’s too the relative compiled directory which will be created and contain compiled plugins, as well as the plugin directory for installation of the plugin, both of which would have to exist in the directory containing the specified plugin source file.

If Compile-SourceScript is going to be easy and predictable to use, then even the parameters themselves should be given names that accurately represent the functionality they serve. As I’ve described earlier, the scripting directory in both amxmodx and sourcemod isn’t simply a directory containing plugin source files, a.k.a. scripts as they would call, but is one containing the compiler and all default include files that come with the mods. With that in mind, a custom scripting directory would actually be one containing those very files themselves. That means, by convention, that the location of the source file cannot actually live outside a scripting directory. Seeing -ScriptingDirectory as separate from the plugin source file and especially allowing specification of the two as separate parameters goes against the convention set by the mods and would very likely come across as confusing to users of this module, as it has been for me.

I'm going to highlight these issues one last time. @leojonathanoh

@leojonathanoh
Copy link
Copy Markdown
Member Author

leojonathanoh commented May 4, 2020

Yes, Compile-SourceScript is itself a convenient wrapper that is designed to support invocation of the two mods’ compiler wrappers; not the most elegant way for compiling plugins, but at least supported by the module that makes the module predictable to use. Yet, what adding -ScriptingDirectory would do is having to prevent compiler wrappers from being invoked unless the directory containing the specified plugin source file is itself like the default scripting directory - that is, containing the compiler, compiler wrapper, and include files, as well as other plugins source files with their include files as well. As though this isn’t confusing enough, there’s too the relative compiled directory which will be created and contain compiled plugins, as well as the plugin directory for installation of the plugin, both of which would have to exist in the directory containing the specified plugin source file.

If arguments for benefits of using Compile-SourceScript as a full replacement wrapper is not enough, then there should also be arguments against supporting native compiler wrappers:

First, the wrappers' behavior are inconsistent across platforms. While it is native to amxmodx and sourcemod, I believe most developers would be frustrated at the difference in behavior between amxmodx's compile.exe vs. compile.sh, where the latter supports flags that former doesn't.

Second, the wrappers do not emit exit codes correctly. It is the responsibility of the native wrapper to collate each compile job's exit code and emit a final batch exit code that represents the fail jobs' exit codes. However compile.exe and compile.sh are not complying to this behavior. As seen in #7 (comment) when using compile.exe, no non-zero exit code is emitted when compilation of any plugin fails. Similarly compile.sh emits the exit code of the last shell statement, which could lead to false positives (i.e. false success): for instance, when there are earlier failed compilation and a successful finalmost compilation, instead of emitting a non-zero exit code, compile.sh emits an exit code of 0. This explains false job positives in #7 (comment) which are mostly due to not getting expected exit codes from native wrappers.

If Compile-SourceScript is going to be easy and predictable to use, then even the parameters themselves should be given names that accurately represent the functionality they serve. As I’ve described earlier, the scripting directory in both amxmodx and sourcemod isn’t simply a directory containing plugin source files, a.k.a. scripts as they would call, but is one containing the compiler and all default include files that come with the mods. With that in mind, a custom scripting directory would actually be one containing those very files themselves. That means, by convention, that the location of the source file cannot actually live outside a scripting directory. Seeing -ScriptingDirectory as separate from the plugin source file and especially allowing specification of the two as separate parameters goes against the convention set by the mods and would very likely come across as confusing to users of this module, as it has been for me.

As I mentioned in #2 (comment) and #2 (comment), the scripting directory contain includes (common dependencies, which in most cases should be unaltered), the compiler and the compiler wrapper, and the plugin source code. However, if the plugin consumes native includes, and does not vendor its own custom includes, then it's source code can live outside of the scripting directory and preferably in its own repo.

@joeltimothyoh joeltimothyoh added the wontfix This will not be worked on label Nov 16, 2021
@joeltimothyoh
Copy link
Copy Markdown
Member

It no longer looks like this feature is any more needed.

Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature wontfix This will not be worked on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add optional param to specify scripting directory

2 participants