-
Notifications
You must be signed in to change notification settings - Fork 1
defold entrypoint #34
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,5 +1,6 @@ | ||||||||||||||||||
| use crate::auth::AuthManager; | ||||||||||||||||||
| use crate::config::{self, WavedashConfig}; | ||||||||||||||||||
| use crate::config::{self, EngineKind, WavedashConfig}; | ||||||||||||||||||
| use crate::dev::entrypoint::{fetch_entrypoint_params, locate_html_entrypoint}; | ||||||||||||||||||
| use crate::file_staging::FileStaging; | ||||||||||||||||||
| use anyhow::Result; | ||||||||||||||||||
| use serde::{Deserialize, Serialize}; | ||||||||||||||||||
|
|
@@ -157,14 +158,39 @@ pub async fn handle_build_push(config_path: PathBuf, verbose: bool, message: Opt | |||||||||||||||||
| anyhow::bail!("No files found in {}", upload_dir.display()); | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| // Get temporary R2 credentials (includes build size) | ||||||||||||||||||
| let engine_kind = wavedash_config.engine_type()?; | ||||||||||||||||||
| let engine_version = wavedash_config.engine_version(); | ||||||||||||||||||
| let entrypoint_params = match engine_kind { | ||||||||||||||||||
| Some(EngineKind::Defold) => { | ||||||||||||||||||
| let html_entrypoint = locate_html_entrypoint(&upload_dir); | ||||||||||||||||||
| let html_path = html_entrypoint.as_deref().ok_or_else(|| { | ||||||||||||||||||
| anyhow::anyhow!("No HTML file found in upload_dir; required for DEFOLD builds") | ||||||||||||||||||
| })?; | ||||||||||||||||||
| let ver = engine_version | ||||||||||||||||||
| .ok_or_else(|| anyhow::anyhow!("DEFOLD engine requires a version"))?; | ||||||||||||||||||
| let html_relative_path = html_path | ||||||||||||||||||
| .strip_prefix(&upload_dir) | ||||||||||||||||||
| .unwrap_or(html_path) | ||||||||||||||||||
| .to_string_lossy() | ||||||||||||||||||
| .replace('\\', "/"); | ||||||||||||||||||
|
Comment on lines
+171
to
+175
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Either canonicalize let upload_dir = upload_dir.canonicalize().with_context(|| {
format!("Failed to canonicalize upload_dir: {}", upload_dir.display())
})?;
// ...
let html_relative_path = html_path
.strip_prefix(&upload_dir)
.with_context(|| format!("HTML entrypoint {} is not under upload_dir {}", html_path.display(), upload_dir.display()))?
.to_string_lossy()
.replace('\\', "/"); |
||||||||||||||||||
| Some( | ||||||||||||||||||
| fetch_entrypoint_params("DEFOLD", ver, html_path, Some(&html_relative_path)) | ||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Engine label hardcoded as The parallel call in
Suggested change
|
||||||||||||||||||
| .await?, | ||||||||||||||||||
| ) | ||||||||||||||||||
|
Comment on lines
+176
to
+179
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Engine label hardcoded as
Suggested change
|
||||||||||||||||||
| } | ||||||||||||||||||
| Some(EngineKind::JsDos | EngineKind::Ruffle | EngineKind::RenPy) => { | ||||||||||||||||||
| wavedash_config.executable_entrypoint_params() | ||||||||||||||||||
| } | ||||||||||||||||||
| _ => None, | ||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wildcard match suppresses compiler exhaustiveness check for future enginesLow Severity The Additional Locations (1)Reviewed by Cursor Bugbot for commit 10f7bb0. Configure here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wildcard
Suggested change
(Or list each variant explicitly. Either way, drop the wildcard.) |
||||||||||||||||||
| }; | ||||||||||||||||||
|
|
||||||||||||||||||
| // Get temporary R2 credentials (includes build size) | ||||||||||||||||||
| let creds = get_temp_credentials( | ||||||||||||||||||
| &wavedash_config.game_id, | ||||||||||||||||||
| engine_kind.map(|e| e.as_label()), | ||||||||||||||||||
| wavedash_config.engine_version(), | ||||||||||||||||||
| engine_version, | ||||||||||||||||||
| wavedash_config.entrypoint(), | ||||||||||||||||||
| wavedash_config.executable_entrypoint_params(), | ||||||||||||||||||
| entrypoint_params, | ||||||||||||||||||
| message.as_deref(), | ||||||||||||||||||
| total_bytes, | ||||||||||||||||||
| &api_key, | ||||||||||||||||||
|
|
||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -14,12 +14,16 @@ struct EntrypointParamsResponse { | |||||||||||||||||||||||||||||||||||||
| entrypoint_params: Value, | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| // Shared by the Godot/Unity/Defold dev and build flows. A root-level `index.html` | ||||||||||||||||||||||||||||||||||||||
| // short-circuits, so single-HTML exports (Godot/Unity) keep their old behavior; the | ||||||||||||||||||||||||||||||||||||||
| // mtime/architecture ranking below only matters for nested multi-HTML dists (Defold). | ||||||||||||||||||||||||||||||||||||||
| pub fn locate_html_entrypoint(upload_dir: &Path) -> Option<PathBuf> { | ||||||||||||||||||||||||||||||||||||||
| let default_index = upload_dir.join("index.html"); | ||||||||||||||||||||||||||||||||||||||
| if default_index.is_file() { | ||||||||||||||||||||||||||||||||||||||
| return Some(default_index); | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| let mut html_files: Vec<PathBuf> = Vec::new(); | ||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The unchanged early-return on lines 17–21 short-circuits the new architecture-aware selection. If For a Defold project migrating onto wavedash, this is a real footgun: any pre-existing root |
||||||||||||||||||||||||||||||||||||||
| for entry in WalkDir::new(upload_dir) | ||||||||||||||||||||||||||||||||||||||
| .min_depth(1) | ||||||||||||||||||||||||||||||||||||||
| .into_iter() | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -28,16 +32,50 @@ pub fn locate_html_entrypoint(upload_dir: &Path) -> Option<PathBuf> { | |||||||||||||||||||||||||||||||||||||
| if entry.file_type().is_file() { | ||||||||||||||||||||||||||||||||||||||
| if let Some(ext) = entry.path().extension() { | ||||||||||||||||||||||||||||||||||||||
| if ext.eq_ignore_ascii_case("html") { | ||||||||||||||||||||||||||||||||||||||
| return Some(entry.into_path()); | ||||||||||||||||||||||||||||||||||||||
| html_files.push(entry.into_path()); | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| None | ||||||||||||||||||||||||||||||||||||||
| html_files.sort_by(|a, b| { | ||||||||||||||||||||||||||||||||||||||
| let modified_a = a.metadata().and_then(|m| m.modified()).ok(); | ||||||||||||||||||||||||||||||||||||||
| let modified_b = b.metadata().and_then(|m| m.modified()).ok(); | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+42
to
+43
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Consider mapping
Comment on lines
+42
to
+43
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Map the failure to a far-past time so missing-mtime files sort to the bottom: let modified_a = a.metadata().and_then(|m| m.modified()).unwrap_or(std::time::SystemTime::UNIX_EPOCH);
let modified_b = b.metadata().and_then(|m| m.modified()).unwrap_or(std::time::SystemTime::UNIX_EPOCH);This also removes the |
||||||||||||||||||||||||||||||||||||||
| let relative_a = a | ||||||||||||||||||||||||||||||||||||||
| .strip_prefix(upload_dir) | ||||||||||||||||||||||||||||||||||||||
| .unwrap_or(a) | ||||||||||||||||||||||||||||||||||||||
| .to_string_lossy() | ||||||||||||||||||||||||||||||||||||||
| .replace('\\', "/"); | ||||||||||||||||||||||||||||||||||||||
| let relative_b = b | ||||||||||||||||||||||||||||||||||||||
| .strip_prefix(upload_dir) | ||||||||||||||||||||||||||||||||||||||
| .unwrap_or(b) | ||||||||||||||||||||||||||||||||||||||
| .to_string_lossy() | ||||||||||||||||||||||||||||||||||||||
| .replace('\\', "/"); | ||||||||||||||||||||||||||||||||||||||
| let architecture_score = |relative: &str| { | ||||||||||||||||||||||||||||||||||||||
| if relative.starts_with("wasm-web/") { | ||||||||||||||||||||||||||||||||||||||
| 0 | ||||||||||||||||||||||||||||||||||||||
| } else if relative.starts_with("js-web/") { | ||||||||||||||||||||||||||||||||||||||
| 1 | ||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||
| 2 | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+54
to
+62
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Defold's HTML5 bundle by default writes to Consider matching anywhere in the path, e.g.
Comment on lines
+54
to
+62
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Match anywhere in the path:
Suggested change
|
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| modified_b | ||||||||||||||||||||||||||||||||||||||
| .cmp(&modified_a) | ||||||||||||||||||||||||||||||||||||||
| .then_with(|| architecture_score(&relative_a).cmp(&architecture_score(&relative_b))) | ||||||||||||||||||||||||||||||||||||||
| .then_with(|| relative_a.cmp(&relative_b)) | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+64
to
+67
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sort key precedence likely inverted. Modification time is the primary key, so the For Defold, both If the intent is "prefer wasm-web, then js-web, then most-recently-modified," the comparator order should be flipped:
Suggested change
Comment on lines
+64
to
+67
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sort key precedence is inverted — architecture preference is effectively dead code.
Flip the comparator so architecture is primary and mtime is the tiebreaker:
Suggested change
(Combine with the |
||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+41
to
+68
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sort comparator does file IO on every comparison — Each invocation of the closure calls Pre-compute keys once with html_files.sort_by_cached_key(|p| {
let relative = p.strip_prefix(upload_dir).unwrap_or(p).to_string_lossy().replace('\\', "/");
let architecture = architecture_score(&relative);
let modified = p.metadata().and_then(|m| m.modified()).unwrap_or(std::time::SystemTime::UNIX_EPOCH);
// Reverse mtime via Reverse() so newest sorts first
(architecture, std::cmp::Reverse(modified), relative)
});This also makes the comparator total/consistent if a file is unlinked mid-sort (the key is captured once at the start). |
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| html_files.into_iter().next() | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| pub async fn fetch_entrypoint_params(engine: &str, engine_version: &str, html_path: &Path) -> Result<Value> { | ||||||||||||||||||||||||||||||||||||||
| pub async fn fetch_entrypoint_params( | ||||||||||||||||||||||||||||||||||||||
| engine: &str, | ||||||||||||||||||||||||||||||||||||||
| engine_version: &str, | ||||||||||||||||||||||||||||||||||||||
| html_path: &Path, | ||||||||||||||||||||||||||||||||||||||
| html_relative_path: Option<&str>, | ||||||||||||||||||||||||||||||||||||||
| ) -> Result<Value> { | ||||||||||||||||||||||||||||||||||||||
| let html_content = fs::read_to_string(html_path) | ||||||||||||||||||||||||||||||||||||||
| .with_context(|| format!("Failed to read {}", html_path.display()))?; | ||||||||||||||||||||||||||||||||||||||
| let api_host = config::get("api_host")?; | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -53,6 +91,7 @@ pub async fn fetch_entrypoint_params(engine: &str, engine_version: &str, html_pa | |||||||||||||||||||||||||||||||||||||
| "engine": engine, | ||||||||||||||||||||||||||||||||||||||
| "engineVersion": engine_version, | ||||||||||||||||||||||||||||||||||||||
| "htmlContent": html_content, | ||||||||||||||||||||||||||||||||||||||
| "htmlPath": html_relative_path, | ||||||||||||||||||||||||||||||||||||||
| })) | ||||||||||||||||||||||||||||||||||||||
| .send() | ||||||||||||||||||||||||||||||||||||||
| .await | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,6 +39,7 @@ struct GamesResponse { | |
| enum EngineType { | ||
| Godot, | ||
| Unity, | ||
| Defold, | ||
| Custom, | ||
| } | ||
|
|
||
|
|
@@ -47,6 +48,7 @@ impl EngineType { | |
| match self { | ||
| EngineType::Godot => "build", | ||
| EngineType::Unity => "build", | ||
| EngineType::Defold => "dist", | ||
| EngineType::Custom => "dist", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Consider defaulting to |
||
| } | ||
| } | ||
|
|
@@ -130,10 +132,25 @@ fn detect_unity(dir: &Path) -> Option<DetectedEngine> { | |
| None | ||
| } | ||
|
|
||
| /// Look for a Defold project marker. | ||
| fn detect_defold(dir: &Path) -> Option<DetectedEngine> { | ||
| if dir.join("game.project").is_file() { | ||
| return Some(DetectedEngine { | ||
| engine_type: EngineType::Defold, | ||
| version_hint: None, | ||
| }); | ||
| } | ||
|
|
||
| None | ||
| } | ||
|
Comment on lines
+136
to
+145
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For Defold, |
||
|
|
||
| fn detect_engine(dir: &Path) -> DetectedEngine { | ||
| if let Some(engine) = detect_godot(dir) { | ||
| return engine; | ||
| } | ||
| if let Some(engine) = detect_defold(dir) { | ||
| return engine; | ||
| } | ||
| if let Some(engine) = detect_unity(dir) { | ||
| return engine; | ||
| } | ||
|
|
@@ -233,6 +250,10 @@ fn generate_toml( | |
| let version = engine_version.unwrap_or("2022.3"); | ||
| toml.push_str(&format!("\n[unity]\nversion = \"{}\"\n", version)); | ||
| } | ||
| EngineType::Defold => { | ||
| let version = engine_version.unwrap_or("1.12.4"); | ||
| toml.push_str(&format!("\n[defold]\nversion = \"{}\"\n", version)); | ||
| } | ||
| EngineType::Custom => { | ||
| toml.push_str("\nentrypoint = \"index.html\"\n"); | ||
| } | ||
|
|
@@ -279,7 +300,7 @@ pub async fn handle_init() -> Result<()> { | |
| let current_dir = std::env::current_dir()?; | ||
| let detected = detect_engine(¤t_dir); | ||
|
|
||
| // Only prompt for version when we detect Godot/Unity. | ||
| // Only prompt for version when we detect a first-class engine. | ||
| // For web builds (threejs, phaser, custom, etc.) no engine config is needed. | ||
| let engine_version: Option<String> = match &detected.engine_type { | ||
| EngineType::Godot => { | ||
|
|
@@ -302,6 +323,13 @@ pub async fn handle_init() -> Result<()> { | |
| Some(version) | ||
| } | ||
| } | ||
| EngineType::Defold => { | ||
| let version: String = cliclack::input("Defold version") | ||
| .placeholder("1.12.4") | ||
| .default_input("1.12.4") | ||
| .interact()?; | ||
| Some(version) | ||
| } | ||
|
Comment on lines
+326
to
+332
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Default Defold version Also note |
||
| EngineType::Custom => None, | ||
| }; | ||
|
|
||
|
|
||


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.
upload_diris not canonicalized here, unlikedev/mod.rs:101. Both call sites computehtml_relative_paththe same way, but onlyhandle_devresolves symlinks/.//..first. Theunwrap_or(html_path)fallback means a strip-prefix failure silently sends the absolute filesystem path to the server ashtmlPath— leaking the local layout and almost certainly producing a 4xx.Easiest fix: canonicalize
upload_dirhere too (mirror thedev/mod.rsblock), or replaceunwrap_orwith an explicit error so a mismatch becomes a clear bug rather than bad data.