Skip to content
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

Add useCow option for Rust #2490

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

surma
Copy link

@surma surma commented Feb 12, 2024

(I am not sure if there’s actually interest in this, but I thought I’d share an idea as a PR and we can take it from here.)

I am using QuickType to generate Rust structs so I can parse a LDTK file (a JSON blob) that is bundled with my binary. Since the JSON blob is already embedded in the binary, it seems wasteful to use String. Instead, for most intents and purposes, it’s functionally identical to use Cow<'_, str> instead of String, but without having to copy all the strings to the heap.

This PR adds a new --use-cow option for Rust, which replaces the occurrences of String> with Cow<'a, str>. Because of the introduction of lifetimes, there is also a bit of machinery as a struct containing a string now needs to have a lifetime itself, and this needs to be recursively propagate up the type tree.

Example before/after Before:
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct World {
    /// Default new level height
    pub default_level_height: i64,

    /// Default new level width
    pub default_level_width: i64,

    /// User defined unique identifier
    pub identifier: String,

    /// Unique instance identifer
    pub iid: String,

    /// All levels from this world. The order of this array is only relevant in
    /// `LinearHorizontal` and `linearVertical` world layouts (see `worldLayout` value).
    /// Otherwise, you should refer to the `worldX`,`worldY` coordinates of each Level.
    pub levels: Vec<Level>,

    /// Height of the world grid in pixels.
    pub world_grid_height: i64,

    /// Width of the world grid in pixels.
    pub world_grid_width: i64,

    /// An enum that describes how levels are organized in this project (ie. linearly or in a 2D
    /// space). Possible values: `Free`, `GridVania`, `LinearHorizontal`, `LinearVertical`, `null`
    pub world_layout: Option<WorldLayout>,
}

After:

#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct World<'a> {    
    /// Default new level height
    pub default_level_height: i64,   
 
    /// Default new level width
    pub default_level_width: i64,

    /// User defined unique identifier
    pub identifier: Cow<'a, str>,

    /// Unique instance identifer
    pub iid: Cow<'a, str>,

    /// All levels from this world. The order of this array is only relevant in
    /// `LinearHorizontal` and `linearVertical` world layouts (see `worldLayout` value).
    /// Otherwise, you should refer to the `worldX`,`worldY` coordinates of each Level.
    pub levels: Vec<Level<'a>>,

    /// Height of the world grid in pixels.
    pub world_grid_height: i64,

    /// Width of the world grid in pixels.
    pub world_grid_width: i64,

    /// An enum that describes how levels are organized in this project (ie. linearly or in a 2D
    /// space). Possible values: `Free`, `GridVania`, `LinearHorizontal`, `LinearVertical`, `null`
    pub world_layout: Option<WorldLayout>,
}

@dvdsgl
Copy link
Member

dvdsgl commented Feb 13, 2024

We really appreciate the contribution, but our CI is in a broken state right now, which prevents us from evaluating new contributions.

@dvdsgl
Copy link
Member

dvdsgl commented Feb 14, 2024

Our CI is fixed! Please rebase.

@surma
Copy link
Author

surma commented Feb 14, 2024

Done :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants