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 parallax support #101

Merged
merged 10 commits into from
Jan 26, 2022
Merged

Add parallax support #101

merged 10 commits into from
Jan 26, 2022

Conversation

perlindgren
Copy link
Contributor

A minor update to support Tiled parallax information.

The asset/tiled_parallax.tmx provides a map with three layers (Background, Middle, and Foreground), each with different parallax information (Middle being the default Tiled (1.0, 1.0)).

The tests/test contains an example:

fn test_parallax_layers() {
    let r = read_from_file_with_path(&Path::new("assets/tiled_parallax.tmx")).unwrap();
    for (i, layer) in r.layers.iter().enumerate() {
        match i {
            0 => {
                assert_eq!(layer.name, "Background");
                assert_eq!(layer.parallax_x, 0.5);
                assert_eq!(layer.parallax_y, 0.75);
            }
            1 => {
                assert_eq!(layer.name, "Middle");
                assert_eq!(layer.parallax_x, 1.0);
                assert_eq!(layer.parallax_y, 1.0);
            }
            2 => {
                assert_eq!(layer.name, "Foreground");
                assert_eq!(layer.parallax_x, 2.0);
                assert_eq!(layer.parallax_y, 2.0);
            }
            _ => panic!("unexpected layer"),
        }
    }
}

The version is bumped to 0.9.6 to indicate the extended information.

Thanks for a great crate by the way!

@bjorn
Copy link
Member

bjorn commented Dec 23, 2021

@perlindgren These changes look good, but unfortunately conflict with the recent reorganization done in c386766. Could you rebase your additions?

Btw, this crate has recently moved to the @mapeditor organization so that we can share maintenance between those that use this crate. Let me know if you're interested in helping with further development!

@aleokdev aleokdev changed the title added parallax support Add parallax support Dec 23, 2021
@PieKing1215
Copy link
Member

Just a note, this doesn't work on object layers as is.
You basically just need to copy the changes into objects.rs as well for it to work.

@perlindgren
Copy link
Contributor Author

So I fixed it, since the changes were so few and small it was actually easier just to copy/paste my changes into the new repo structure (mainly its in the layers.rs.

@bjorn did you have anything in particular in mind regarding the repo development. I played around a bit with Rust Bevy, and implemented parallax scrolling there, but since the rs-tiled crate did not support parallax info extraction I just went for it.

Copy link
Contributor

@aleokdev aleokdev left a comment

Choose a reason for hiding this comment

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

Pretty good, there are just a few minor formatting issues.

src/layers.rs Outdated Show resolved Hide resolved
src/layers.rs Outdated Show resolved Hide resolved
@bjorn
Copy link
Member

bjorn commented Dec 29, 2021

So I fixed it, since the changes were so few and small it was actually easier just to copy/paste my changes into the new repo structure (mainly its in the layers.rs).

That's great, thanks! What about also adjusting objects.rs as per @PieKing1215's comment? And of course these attributes also affect ImageLayer.

Or, rather than duplicating more stuff, we could merge this as-is for now (just addressing the minor code style adjustments) and instead look into refactoring the data structure into an enum? I think we can pretty much copy the structs from https://gitlab.com/flukejones/tiled-json-rs/-/blob/master/src/layer.rs#L79-112.

@bjorn did you have anything in particular in mind regarding the repo development. I played around a bit with Rust Bevy, and implemented parallax scrolling there, but since the rs-tiled crate did not support parallax info extraction I just went for it.

We'll need to make sure this crate supports all features of TMX and preferably also supports deserializing JSON. Secondly I think we need to make it more convenient to use, while keeping an eye on performance. Is that what you were asking?

@bjorn bjorn requested a review from aleokdev January 24, 2022 15:10
@bjorn
Copy link
Member

bjorn commented Jan 24, 2022

I've addressed the minor code style issues and merged master again. I think this can be squashed and merged now.

Regarding adding these to ObjectGroup and ImageLayer, I think we can do it in a separate change as mentioned earlier.

Copy link
Contributor

@aleokdev aleokdev left a comment

Choose a reason for hiding this comment

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

Add changelog changes and it's good to go

@bjorn
Copy link
Member

bjorn commented Jan 24, 2022

I'm not sure why those newlines in assets/tiled_parallax.tmx changed again...

@bjorn bjorn requested a review from aleokdev January 24, 2022 16:41
aleokdev
aleokdev previously approved these changes Jan 26, 2022
@aleokdev
Copy link
Contributor

Sorry for being so late, the notification didn't get through.

Conflicts:
	src/layers.rs
@bjorn bjorn merged commit 75e0869 into mapeditor:master Jan 26, 2022
@bjorn
Copy link
Member

bjorn commented Jan 26, 2022

Sorry for being so late, the notification didn't get through.

No problem, it was good to wait with this until you were done with #131. :-)

@aleokdev aleokdev added this to the 0.10.0 milestone Mar 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants