Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions .devcontainer/devcontainer.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// For format details, see https://aka.ms/devcontainer.json. For config options, see the
// README at: https://github.com/devcontainers/templates/tree/main/src/rust
{
"name": "Rust",
// Or use a Dockerfile or Docker Compose file. More info: https://containers.dev/guide/dockerfile
"image": "mcr.microsoft.com/devcontainers/rust:1-1-bullseye",

// Features to add to the dev container. More info: https://containers.dev/features.
// "features": {},

// Configure tool-specific properties.
"customizations": {
// Configure properties specific to VS Code.
"vscode": {
"settings": {},
"extensions": [
"streetsidesoftware.code-spell-checker"
]
}
},

// Use 'forwardPorts' to make a list of ports inside the container available locally.
// "forwardPorts": [],

// Use 'postCreateCommand' to run commands after the container is created.
// "postCreateCommand": "rustc --version",
"postCreateCommand": "sudo apt update && sudo apt install -y git-lfs && git lfs install"

// Uncomment to connect as root instead. More info: https://aka.ms/dev-containers-non-root.
// "remoteUser": "root"
}
11 changes: 11 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ members = [
"spfc-abi",
"spfc-cli",
"backends/spfc-target-ttf"
, "spfc-macros"]
, "spfc-macros", "backends/spfc-target-p8"]

[workspace.package]
version = "0.0.1"
Expand All @@ -15,3 +15,5 @@ license = "Apache-2.0"
# Share dependencies across the workspace to compile faster
[workspace.dependencies]
anyhow = "1.0"
spf = { version = "0.8.0-alpha.0", default-features = false, features = ["std"] }
render_spf = { git = "https://github.com/SimplePixelFont/render-spf.git" }
14 changes: 14 additions & 0 deletions backends/spfc-target-p8/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[package]
name = "spfc-target-p8"
version = "0.1.0"
edition.workspace = true

[lib]
crate-type = ["cdylib"]

[dependencies]
spfc-abi = { path = "../../spfc-abi" }
spf.workspace = true
bitvec = "1.0.1"
render_spf.workspace = true
anyhow.workspace = true
15 changes: 15 additions & 0 deletions backends/spfc-target-p8/src/builders/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
use std::collections::BTreeMap;

use crate::utilities::PixmapGlyph;

pub mod writer;
pub use writer::*;


#[derive(Default, Debug)]
pub struct Process {
pub family_name: String,
pub family_version: f64,
pub manufacturer: String,
pub pixmap_pairs: BTreeMap<String, PixmapGlyph>,
}
38 changes: 38 additions & 0 deletions backends/spfc-target-p8/src/builders/writer.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
use crate::{builders::Process, utilities::{last_character_index, max_height, max_width}};
use anyhow::Result;

pub fn create_program_string(process: &Process) -> Result<String> {
let mut program_string = String::new();

program_string += "pico-8 cartridge // http://www.pico-8.com\nversion 41\n__lua__\n";

program_string += format!("-- {} v{}\n-- {}\n-- Generated by spfc\n-- https://github.com/SimplePixelFont/spfc\n\n", process.family_name, process.family_version, process.manufacturer).as_str();

let font_function = format!("load_{}", process.family_name.to_ascii_lowercase().replace(" ", "_"));
program_string += format!("-- Copy the function below\n-- into your own cart\nfunction {}()\n\t-- enable custom fonts\n\tpoke(0x5f58,0x81)\n\n\t-- add font to memory\n", font_function).as_str();
Comment on lines +9 to +12
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Sanitize font metadata before embedding it into Lua.

family_name and manufacturer are spliced into a comment, a function name, and a quoted string verbatim. Quotes, newlines, punctuation, or a leading digit can generate invalid Lua, and crafted metadata can inject extra code into the emitted cart.

Also applies to: 34-35

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backends/spfc-target-p8/src/builders/writer.rs` around lines 9 - 12, Sanitize
process.family_name and process.manufacturer before splicing into program_string
and when building font_function: create a safe comment string by
stripping/normalizing newlines and removing any "--" sequences and control chars
(replace with spaces), create a safe identifier for font_function by
to_ascii_lowercase(), replacing any non [a-z0-9_] with "_" and prefixing (e.g.,
"f_") if it would start with a digit, and create a safe quoted string by
escaping backslashes and quotes and replacing newlines with "\n"; then use those
sanitized values instead of raw process.family_name/process.manufacturer when
constructing program_string and font_function (references: program_string,
font_function, load_... function).



let mut buffer = vec![0; (last_character_index(&process.pixmap_pairs) as usize + 1) * 8];
buffer[0] = max_width(&process.pixmap_pairs) + 1; //1 added for letter spacing
buffer[1] = max_width(&process.pixmap_pairs); // This will be added as a custom flag
buffer[2] = max_height(&process.pixmap_pairs) + 1; //1 added for line spacing
buffer[3] = 0; // This will be added as a custom flag offset_x
buffer[4] = 0; // This will be added as a custom flag offset_y

for (key, glyph) in &process.pixmap_pairs {
if let Some(p8_char) = crate::utilities::get_character_by_symbol(key) {
let char_index = p8_char.id as usize;
let bitmap_data = &glyph.bitmap;
for (i, byte) in bitmap_data.iter().enumerate() {
buffer[char_index * 8 + i] = *byte;
}
}
}
Comment on lines +15 to +30
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fail fast on unsupported glyph symbols instead of silently dropping them.

If a glyph key is not present in P8SCII_CHARSET, last_character_index(...) effectively ignores it and this loop just skips writing it. The result is a “successful” cart with missing characters and no signal to the caller. Please reject unsupported symbols before allocating/writing the buffer.

Suggested direction
+    let unsupported: Vec<_> = process
+        .pixmap_pairs
+        .keys()
+        .filter(|key| crate::utilities::get_character_by_symbol(key).is_none())
+        .cloned()
+        .collect();
+
+    if !unsupported.is_empty() {
+        anyhow::bail!(
+            "unsupported Pico-8 glyphs: {}",
+            unsupported.join(", ")
+        );
+    }
+
     let mut buffer = vec![0; (last_character_index(&process.pixmap_pairs) as usize + 1) * 8];
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backends/spfc-target-p8/src/builders/writer.rs` around lines 15 - 30, The
loop currently skips glyphs whose keys are not in P8SCII
(crate::utilities::get_character_by_symbol), leading to silent missing
characters; before allocating the buffer or writing (i.e., before the call that
uses last_character_index(&process.pixmap_pairs) and before filling buffer),
iterate process.pixmap_pairs and validate every key via get_character_by_symbol,
and if any validation returns None immediately return an Err (or propagate an
error) naming the unsupported symbol so the caller fails fast; only proceed to
buffer allocation and the existing write loop after all keys are confirmed
valid.


program_string += format!("\tpoke(0x5600,unpack(split\"{}\"))\nend\n\n", buffer.iter().map(|byte| byte.to_string()).collect::<Vec<String>>().join(",")).as_str();

program_string += format!("-- Sample usage below\nfunction _init()\n\t{}()\nend\n\n", font_function).as_str();
program_string += format!("function _draw()\n\tcls()\n\tprint(\"Hello from {}!\")\nend ", process.family_name).as_str();

Ok(program_string)
}
46 changes: 46 additions & 0 deletions backends/spfc-target-p8/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
use spf::core::layout_from_data;
use spfc_abi::{BackendInfo, CURRENT_ABI_VERSION, CompileOptions, CompileResult, PluginOption};

mod builders;
use builders::*;
mod utilities;
use utilities::*;

#[spfc_abi::export]
fn get_backend_info() -> BackendInfo {
BackendInfo {
name: "P8 PICO8 Backend",
version: 1,
abi_version: CURRENT_ABI_VERSION,
}
}

#[spfc_abi::export]
fn get_plugin_options() -> Vec<PluginOption> {
vec![]
}

#[spfc_abi::export]
fn compile(options: CompileOptions) -> CompileResult {
let data = std::fs::read(&options.input).unwrap();
let layout = layout_from_data(&data).unwrap();
let font_table = layout.font_tables.first().unwrap();
let font = font_table.fonts.first().unwrap();
Comment on lines +27 to +28
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Avoid implicit “first font” selection.

Line 27 and Line 28 silently pick the first table/font. For inputs containing multiple fonts, this can compile the wrong asset with no signal. Please validate the expected cardinality (or add explicit selection via plugin options) and fail clearly when ambiguous.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backends/spfc-target-p8/src/lib.rs` around lines 27 - 28, The code currently
picks layout.font_tables.first() and font_table.fonts.first() without
validation; update the selection logic in lib.rs to validate cardinality or
accept an explicit selection option: check layout.font_tables.len() and
font_table.fonts.len(), and if >1 either return a clear error (including counts
and available identifiers) or use a provided plugin/config option to choose the
desired table/font (by index or name); reference the symbols layout.font_tables,
font_table.fonts, font_table, and font when adding these checks and error
messages so ambiguous inputs fail loudly instead of silently selecting the first
item.


let mut process = Process::default();
process.family_name = font.name.clone();
process.family_version = font.version as f64;
process.manufacturer = font.author.clone();
process.pixmap_pairs = create_pixmap_pairs(&layout);

let font_data = create_program_string(&process).unwrap();
std::fs::write(&options.output, &font_data).unwrap();
Comment on lines +25 to +37
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Remove panic paths from the exported compile entrypoint.

Line 25–Line 37 use unwrap() repeatedly on I/O, parse, lookup, generation, and write. Any bad input or FS failure will panic instead of returning a controlled CompileResult, which makes this backend brittle in host environments.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backends/spfc-target-p8/src/lib.rs` around lines 25 - 37, The current compile
entrypoint unwraps I/O and parse operations (std::fs::read, layout_from_data,
font_table/font lookup, create_program_string, std::fs::write) which will panic;
replace each unwrap with proper error handling that converts failures into a
controlled CompileResult (or the crate's error type) and returns early.
Specifically, validate the file read (options.input), layout_from_data(error),
presence checks on layout.font_tables.first() and font_table.fonts.first(), the
create_program_string result, and the write to options.output; map each failure
to a clear CompileResult error variant or message and propagate (use ? or
explicit map_err) instead of panicking so Process, create_pixmap_pairs, and
downstream callers never cause a panic.


println!(
"Finished writing {} bytes to {}",
font_data.len(),
options.output
);

CompileResult::Success
}
Loading