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
Implement transpiler support #8833
Conversation
35a435e
to
f15c09b
Compare
This changeset adds initial Bob and Editor support for plugins that transpile 3rd-party languages to Lua. Transpiler plugins need to implement an `ILuaTranspiler` interface. See [the Teal extension](https://github.com/defold/extension-teal) for reference implementation. Fixes #8291
(:require [clojure.string :as string] | ||
[dynamo.graph :as g] | ||
[editor.build-target :as bt] | ||
(:require [dynamo.graph :as g] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved script compilation to a different file (script_compilation.clj
)
:modules preprocessed-modules | ||
:proj-path (resource/proj-path resource)} | ||
:deps preprocessed-go-prop-dep-build-targets | ||
:dynamic-deps (mapv lua/lua-module->path preprocessed-modules)})])))))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
99% of the code in this file is taken from script.clj
. There is a small behavior difference: module dependencies are now :dynamic-deps
that only reference dependent file paths instead of including the dependent build targets.
(defn reload-plugins! [project touched-resources] | ||
(g/with-auto-evaluation-context evaluation-context | ||
(let [workspace (workspace project evaluation-context) | ||
code-preprocessors (workspace/code-preprocessors workspace evaluation-context) | ||
code-transpilers (code-transpilers project)] | ||
(workspace/unpack-editor-plugins! workspace touched-resources) | ||
(code.preprocessors/reload-lua-preprocessors! code-preprocessors workspace/class-loader) | ||
(code.transpilers/reload-lua-transpilers! code-transpilers workspace workspace/class-loader) | ||
(workspace/load-clojure-editor-plugins! workspace touched-resources)))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved plugin reload from workspace to project because it's needed for transpilers.
(defn flatten-build-targets | ||
"Breadth first traversal / collection of build-targets and their child :deps, | ||
skipping seen targets identified by the :content-hash of each build-target." | ||
([build-targets] | ||
(flatten-build-targets build-targets #{})) | ||
([build-targets seen-content-hashes] | ||
(assert (set? seen-content-hashes)) | ||
(loop [targets build-targets | ||
queue [] | ||
seen seen-content-hashes | ||
result (transient [])] | ||
(if-some [target (first targets)] | ||
(let [content-hash (:content-hash target)] | ||
(assert (bt/content-hash? content-hash) | ||
(str "Build target has invalid content-hash: " | ||
(resource/resource->proj-path (:resource target)))) | ||
(if (contains? seen content-hash) | ||
(recur (rest targets) | ||
queue | ||
seen | ||
result) | ||
(recur (rest targets) | ||
(conj queue (flatten (:deps target))) | ||
(conj seen content-hash) | ||
(conj! result target)))) | ||
(if-some [targets (first queue)] | ||
(recur targets | ||
(rest queue) | ||
seen | ||
result) | ||
(persistent! result)))))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to editor/build.cj
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks mostly good to me, but there are some open questions and concerns.
com.dynamo.cr/com.dynamo.cr.bob/src/com/defold/extension/pipeline/ILuaTranspiler.java
Show resolved
Hide resolved
com.dynamo.cr/com.dynamo.cr.bob/src/com/defold/extension/pipeline/ILuaTranspiler.java
Outdated
Show resolved
Hide resolved
com.dynamo.cr/com.dynamo.cr.bob/src/com/defold/extension/pipeline/ILuaTranspiler.java
Outdated
Show resolved
Hide resolved
com.dynamo.cr/com.dynamo.cr.bob/src/com/defold/extension/pipeline/ILuaTranspiler.java
Outdated
Show resolved
Hide resolved
com.dynamo.cr/com.dynamo.cr.bob/src/com/defold/extension/pipeline/ILuaTranspiler.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I made a note about a code comment that could be potentially helpful.
:ddf-type AtlasProto$Atlas | ||
:load-fn load-atlas | ||
:icon atlas-icon | ||
:icon-class :design |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly indentation, but also added :icon-class
:ddf-type GameSystem$CollectionProxyDesc | ||
:load-fn load-collection-proxy | ||
:icon collection-proxy-icon | ||
:icon-class :property |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added :icon-class
:load-fn load-collision-object | ||
:sanitize-fn sanitize-collision-object | ||
:icon collision-object-icon | ||
:icon-class :design |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added :icon-class
(code-transpilers project) | ||
node-id | ||
(resource/proj-path resource))] | ||
(cond-> load-tx-steps transpiler-tx-steps (concat transpiler-tx-steps))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After addressing your concerns, the performance looks like this:
project size | total load time (ms) | transpiler overhead (ms) | decline (%) |
---|---|---|---|
small | 724.76 | 0.80 | 0.07% |
big (without transpilers) | 179477.23 | 19.34 | 0.01% |
big (with transpilers) | 174382.14 | 27.21 | 0.02% |
I think this is fine!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to merge! I really like the addition of :icon-class
to the resource types. ❤️
This changeset adds initial Bob and Editor support for plugins that transpile 3rd-party languages to Lua. Transpiler plugins need to implement an
ILuaTranspiler
interface. See the Teal extension for reference implementation.Fixes #8291