godot-nim / gdext-nim

Nim for Godot GDExtension. A pure library and a CLI tool.
MIT License
46 stars 5 forks source link

Potential exported library-specific annoyances #30

Open insomniacUNDERSCORElemon opened 3 months ago

insomniacUNDERSCORElemon commented 3 months ago

Context:

Particularly on Linux, files without extensions are often executables and thus you end up having the same icon as the main executable.

Additionally, when exporting the libraries, they are put into the same directory as the main executable.

Further potential for project-end-user confusion if the library and executable both use the project name, and annoyance if a project uses a lot of add-ons.


OS-specific library extensions can easily be done in the .gdextension files, however: it must also be done in the .nimble files with the bin line, which is not OS specific* and will break the project if you don't add the same extension that is set for your current platform.

These issues are low-priority, but should also be addressed in the template/demos/wiki to prevent this from being an issue. I think it is also good to rename the library/bin lines from projectname to projectname_core or something like that, particularly if library extensions are not used.

*=(to my knowledge at least, but if it's possible by duplicating the same multi-line approach of the .gdextension file that's not so great long-term, particularly as this is per-addon.)


Task list:

insomniacUNDERSCORElemon commented 3 months ago

On a related note, HTML5? I enabled the Extensions Support option and I get the error No suitable library found for GDExtension with export and with remote debug option (which loads but obviously not the code).

panno8M commented 3 months ago

On a related note, HTML5? I enabled the Extensions Support option and I get the error No suitable library found for GDExtension with export and with remote debug option (which loads but obviously not the code).

try below:

  # PROJECT.gdextension
+ web.debug.wasm32 = "res://path/to/wasm32"
  # Probably not necessary, but if it doesn't work, try it.
+ web.release.wasm32 = "res://path/to/wasm32"
+ web.debug.threads.wasm32 = "res://path/to/wasm32"
+ web.release.threads.wasm32 = "res://path/to/wasm32"

EDIT: It looks like you will probably have to build the extension into a wasm format. I am looking into how to do this.

EDIT 2: Use emcc (Emscripten Compiler Frontend) instead of gcc. godot-cpp seems to work that way.

insomniacUNDERSCORElemon commented 3 months ago

Use emcc (Emscripten Compiler Frontend) instead of gcc. godot-cpp seems to work that way.

I don't know if it is useful here, it's not Godot but it is Nim: Naylib's webassembly instructions.

Though by the point it comes to needing a guide, my passing interest is mostly gone.

EDIT: Yeah it's the same thing, I had Naylib's Platform Support table scrolled to by coincidence.

Also I just looked at what Godot exported for my project, I don't think any of it's my code but there's a 3.1MiB .js file with "the longest of those lines is 260,661 characters long" autowrapped to 55K lines

panno8M commented 3 months ago

how scary it is

insomniacUNDERSCORElemon commented 2 months ago

I just had a thought: if changing compilers is part of the (non-standard) export process, this makes multiple add-ons even more tedious.

Likely there should be a default_config.nims file in the add-ons folder to cover all add-ons so config.nims is optional. Nice too if the add-ons override the options individually (so only differing lines are needed) with some option to ignore defaults.

Side note: I suspect some usage of projectName should likely be replaced with AddOnName, for instance with switch("define", "projectName:" & projectName()). EDIT: I see this is Nim-specific naming, a bit confusing

panno8M commented 2 months ago

In this regard, I don't think we should bother to define separate extensions made with Nim. For example, it could be structured as follows:

# nimextension1.nim
import gdext
import nimextension1/[ class1, class2 ]
export class1, class2

proc register* =
  register Class1
  register Class2
# entry.nim.cfg
--path: "nimextension1/src"
--path: "nimextension2/src"
# entry.nim
import nimextension1
import nimextension2

process initialize_scene:
  nimextension1.register()
  nimextension2.register()

GDExtension_EntryPoint init_library

In this way, you can manage extensions in the same way as nimble packages.

insomniacUNDERSCORElemon commented 2 months ago

In this regard, I don't think we should bother to define separate extensions made with Nim

Shouldn't be needed yeah, but I was thinking that there might be actual reasons to do like:


Some of those changes you listed seem like a step backwards to me. Like the fact that you'd need to manually edit another file to load add-ons that are well-defined.

The duplicated names are a bit maddening. I know it's not actually required (currently) but it's annoying to find out which names are and aren't linked/required (and which exact name it is referring to at-a-glance) when you need to rename all of these things. At least when starting out.

The extra folder isn't great either, I prefer the current structure. Even then I could see one less folder being viably used (whatever desired subfolders added by the add-on developers) if classes could instead be defined in the .gdextension file instead (or whatever file is used to generate it), which would also cut down on number of needed places to edit.

Alternatively, could this be handled automatically by the nim file where the class is already defined (extra code or a pragma)?

panno8M commented 2 months ago

I have suggested a redundant configuration here because, as noted above, it is to be the same configuration as the nimble package. This way, when gdext is stable and registered in the package repository in the future, each extensions can be published individually via nimble publish.

The limitation of duplicated class names is considered a rather necessary restriction, since godot does not allow the registration of duplicate classes.

insomniacUNDERSCORElemon commented 2 months ago

This way..each extensions can be published individually via nimble publish.

I am not sure if I see the appeal for myself. At least I don't think most things I make will make sense to publish just the code side of.

This sounds better if you could requires a package and have the classes available (ready to add node(s) to scene) after building without needing to deal with files or additional configuration.

Such as for:

Even then this seems like it may be inflexible when it comes to scenes/assets, and that something like Godot's own assetlib may make a better choice for distribution.

The limitation of duplicated class names is considered a rather necessary restriction, since godot does not allow the registration of duplicate classes.

I was thinking that excluding certain characters would prevent that from happening in most case, such as parenthesis would prevent (copy 1)/(2)_copy as that would likely be the most common occurrence of duplicate classes. Aside from preventing accidental stuff like that/other quality-of-life stuff with folder management, just fail the build.

EDIT: Though what I was saying about duplicate names was not about class names, but about using the same name over-and-over for files/folders when it isn't needed from a technical perspective. Though I could see that being part of the nimble stuff you mentioned, as I don't see support for an explicit definition of name/packageName with nimble that might make that less of an issue.

panno8M commented 2 months ago

I am not sure if I see the appeal for myself. At least I don't think most things I make will make sense to publish just the code side of.

So, that feature will be mostly for extension developers, not game developers. Everything in config.nims is only needed for the build. There is no need to have it in each package. It is true that with nimble it is difficult to bundle tscn and other resources in a package, but with atlas it is possible.

Ultimately, gdext would be installed in the global environment using nimble install and the distributed extensions would be deployed project-local using atlas use.

Regarding assetlib, the average Godot user will need to set up a Nim development environment in order to use extensions made with Nim. That means which is a non-functional asset for the majority of Godot users.

The asset must work. If the asset doesn't run or otherwise doesn't work in the specified Godot version, then it will be rejected. Godot Engine 4.2 documentation - Submitting to the Asset Library

I was thinking that excluding certain characters would prevent that from ...

I'm sorry. I did not understand what this paragraph meant. At least, parentheses and other symbols cannot be included in the module name (Nim file name) or in the path to the module.

insomniacUNDERSCORElemon commented 2 months ago

I just noticed library name is covered by bootstrapconf.nims, what I said about using filenames still holds true.

For instance if you could do something like this with a .cfg file:

--libraryName: "$self"

So banana_rpg.cfg would result in banana_rpg.so and I assume $workspace.gdextension with the proper library names.

That way the .nims could be optional per-extension (in base project or addons folder as I've said before, I tested and it does work like this with the switch statements). And obviously, easier changes.

panno8M commented 2 months ago

Let's clear things up a bit, as I feel there are some discrepancies in the assumptions.

Suppose the project structure looks like this.

When you run nim c project/workspaceA/bootstrap here, there are only two files, project/config.nims and workspaceA/config.nims, that are referenced. The config.nims belonging to the extension are not referenced in the first place. Currently prohibits building without going through bootstrap, so extensionA1/config.nims has no meaning.

panno8M commented 2 months ago

I'm sorry, but I can't understand it well with a textual explanation. It would be easier to understand if you could show us some kind of diagram or pseudo code.

insomniacUNDERSCORElemon commented 2 months ago

I am talking more about bootstrapconf.nims with this line:

switch("out", "$projectdir/lib"/("workspaceentry".toDll)) (I tried adding a $ and again also with camelCase, didn't make it dynamic if that was your intent)

EDIT: A more straightforward approach than what I've suggested before would be to just set the library name in bootstrap.nim, as that is more obvious and more in one place. @panno8M does that make sense+is it viable?

panno8M commented 2 months ago

If we change bootstrapconf.nims like this, the dynamic library names will be in sync with those in your workspace.

# bootstrapconf.nims
let workspaceName = projectDir().splitFile.name

switch("define", "projectName:" & workspaceName)
switch("out", "$projectdir/lib"/workspaceName.toDll)

What you want to do is change the dynamic library name regardless of the workspace name?

does that make sense+is it viable?

Don't. This can be accomplished by specifying --libraryName:foo in workspace/config.nims or bootstrap.nim.cfg.

insomniacUNDERSCORElemon commented 2 months ago

What you want to do is change the dynamic library name regardless of the workspace name?

Yes it would be nice, particularly in the context of the library being exported at the same level as the executable (not in a lib folder).

Admittedly maybe it isn't the most important thing but I have a lot of little thoughts on workflow and potential issues.

Details

I could see folder renaming causing issues with file history (locally) and git, though that matters more over multiple renames, such as a working title. Internal naming could be used (update codenames, development cycle) in the workspace that aren't intended to be visible to end-users. This matters more for teams. The other side of the coin, maybe the library naming could reflect different stages/releases of a game that has a continuous workflow... allowing files to coexist in the same directory for a user, particularly for accessing user data. Or even completely different games (perhaps by the same maker) that use the same library name could cause an issue. The longer a project name, the more annoying it will be to have it duplicated as the workspace name.


Perhaps these are all future problems and it doesn't make sense for it to bother me so much.

Though I like the idea of bootstrap.nim being The configuration file, especially if custom builds (switching classes/extensions on/off or for alternate versions) could be done via it. Though perhaps even that can be smarter handled at runtime with existing capability to allow turning groups of classes off or swapping them out with other ones in one library or via a backup library.

Note that in my previous comment I deleted some context, one was a response on config.nims and that I know it only applies to builds/not bootstrapless (and that I was thinking one file for all libraries OR 1 file for main library and another as the default for all other addons etc).

insomniacUNDERSCORElemon commented 2 months ago

This can be accomplished by specifying --libraryName:foo in workspace/config.nims or bootstrap.nim.cfg.

I tried this and it didn't work. invalid command line option: '--libraryName'

This was also after updating.

Change projectName to be taken from the Nim build system, so that it does not need to be specified in bootstrapconf.nims.

This didn't seem to work for me either. I mean the code above yes, but I still needed the line let workspaceName = projectDir().splitFile.name. in bootstrapconf

insomniacUNDERSCORElemon commented 1 week ago

@panno8M I mentioned the name change in another issue, but I just thought to try this with exporting and it does indeed seem to cause an issue (as I've added to the task list). The old way gave libraries extension-specific names instead so there was no chance for name conflict.

Aside from that, I assume an error on export or possibly merging libraries (maybe even re-compile in the future, particularly for release version?) could potentially be options? I mean merging might not be great for users (for example if they want to not have a specific optional feature, or drop-in updates), but I guess that depends on context.

EDIT: Or instead just keeping their load order and then renaming (_0-_3 etc).

Logically, I'd expect a fully modular full-featured game to have libraries split in this sort of order:

Which would certainly be easier with the previous naming behavior. (though most games probably only need 1 library, maybe a second for optional/user content).

Also, speaking of this split... I should note that the missing library does cause a segfault. My whole idea here is that load order is a hierarchy (though importance could be configured too). Or is this a gd-extension limitation?

EDIT: And with this same line of thinking, core might be compiled as release but anything else might not be (because it's being iterated on, does not need the performance, or is otherwise not proven) so in those cases it is useful to not be marked as release or debug (if exporting is going to ignore the wrong type).

panno8M commented 1 week ago

I assume an error on export or possibly merging libraries (maybe even re-compile in the future, particularly for release version?) could potentially be options?

I can't imagine what these keywords mean, can you elaborate a bit more?


There seems to be a little discrepancy between our perceptions of GDExtension.

The GDExtensions are independent of each other, and it is basically not possible, for example, to use classes defined in one extension from in the other. GDExtension is literally extensions to Godot itself. Therefore, the loading order of the GDExtension does not seem to make sense.

insomniacUNDERSCORElemon commented 1 week ago

I can't imagine what these keywords mean, can you elaborate a bit more?

I am not sure which words you mean.

For the bit you quoted, the only thing I could add would be to say that if someone had a large game already they might want to create new content as another library at first to avoid recompiling the main library frequently. Even if they don't want/need the game to be distributed in this manner.

EDIT: Though having a beta library that allows the user to play the stable base game (either by turning the beta off with a setting, split content/game modes/saves etc, or deleting the library) would be great.

and it is basically not possible, for example, to use classes defined in one extension from in the other

That is unfortunate as I do think that makes sense. New/optional content is often a new package that relies on the base game, and may even require functionality from tools like a level editor that the base game doesn't require (that is why I ordered the list like that).

Either way though, I'd say missing libraries should not prevent the game from launching unless it's the main library. Particularly with what you've said, libraries (and the main one most of all) should be assumed to have functionality all on their own.

EDIT: Also to be clear, even ignoring all of this the old way of naming libraries via extension name was much better. Even if the new way didn't create duplicates.

panno8M commented 6 days ago

This is a bit of a roundabout approach, but I will try to solve this problem by allowing the bootstrap.nim to be freely named.

In fact, at this point there is already nothing restricting this name other than it being a keyword in the gdextwiz build.

Now we just need to figure out how to specify the build target to gdextwiz, and I'll get started when I have a better idea.

insomniacUNDERSCORElemon commented 6 days ago

This is a bit of a roundabout approach, but I will try to solve this problem by allowing the bootstrap.nim to be freely named

That's overcomplicating it IMO, why was it even changed from libextensionname (from folder name) to libbootstrap? It seems like purely a step backwards, just as a dynamic build target would be.

panno8M commented 6 days ago

I certainly acknowledge that this is a setback. The reason is that I still have doubts about the current project structure, and I wanted to avoid automation that would push for a fixing structure until it is in a form that makes sense to me. I also think that this is not too much of a problem, since the conventional functionality can be reproduced by the user by combining the --out and --outdir flags.

insomniacUNDERSCORElemon commented 6 days ago

I do think using workspace names (which I should've said before) is intuitive and great for 95%+ of use-cases so if users need to rename this manually that's not great. And as I've said, I don't like configuration in too many places.

Renaming seemed to me like something that could wait for a while if it's even needed/requested. And even then, I would say maybe define the library name via the pragma in the bootstrap file (like {.execon: initialize_scene, libname: "mygame".} would give libmygame.so... otherwise optional or templated to using $workspace as the argument).