Add SFML example #133

aleokdev · 2022-01-25T22:38:21Z

Adds a rough demo using SFML for rendering. Closes #35.

Extremely WIP. Right now the example is really long because a lot of boilerplate is required. If you realize any way to reduce this boilerplate without making the example more specific, let me know.

Adds a rough demo using SFML for rendering.

bjorn

Works fine! Though we really need a nicer example map I guess. :-)

I've left a number of comments, most of which I'll also address by pushing my changes to the example, but let me know if you disagree about any of them.

It took me a bit to figure out the dependencies to install. In the end I had to install libcsfml-dev, libsfml-dev, build-essential (not entirely sure) and clang before it would work. Probably something we need to document somewhere.

Cargo.toml

examples/sfml/tilesheet.rs

examples/sfml/mesh.rs

bjorn · 2022-01-26T10:04:21Z

I've left a number of comments, most of which I'll also address by pushing my changes to the example, but let me know if you disagree about any of them.

Hmm, so I've already addressed most of my comments, since I anyway wanted to make sure the changes made sense, given that I'm still learning Rust. However, it appears I can't push to your branch. I've pushed the change here instead: bjorn@b547ffa

aleokdev · 2022-01-26T10:07:56Z

It took me a bit to figure out the dependencies to install. In the end I had to install libcsfml-dev, libsfml-dev, build-essential (not entirely sure) and clang before it would work. Probably something we need to document somewhere.

The sfml crate is supposed to vendor its own version of sfml, but for some reason still requires those packages as dependencies. I've had issues with that as well in the past, I'll create an issue in the sfml-rs repo.

bjorn · 2022-01-26T10:15:50Z

The sfml crate is supposed to vendor its own version of sfml, but for some reason still requires those packages as dependencies. I've had issues with that as well in the past, I'll create an issue in the sfml-rs repo.

Hmm, according to their documentation they expect you to just install SFML and a suitable compiler first.

aleokdev · 2022-01-26T10:38:00Z

Hmm, according to their documentation they expect you to just install SFML and a suitable compiler first.

See this PR. I don't think that should apply any more, at least on master. (0.16 does not vendor CSFML).

bjorn · 2022-01-26T11:21:47Z

I just realized that the autobuild is failing because of the SFML dependency, even though actually it is only used by one of the examples, which isn't actually being compiled. Is there any way to make this dependency only install when trying to compile that example?

bjorn · 2022-01-26T11:23:51Z

@alexdevteam Would you mind allowing me to push into your branch (the pull request setting), so that I could just push bjorn@b547ffa?

aleokdev · 2022-01-26T11:27:14Z

Sure, but I can't find the option :(
I've already looked here, but there's nothing in that place. Where is the setting?

bjorn · 2022-01-26T11:31:01Z

I've already looked here, but there's nothing in that place. Where is the setting?

It should be exactly there, so not sure why you can't see it. Maybe you could just cherry-pick this change for now.

aleokdev · 2022-01-26T11:34:40Z

Okay there we go, for some reason the setting isn't visible if the repo is in an organization. Moved it to my account and now you should be able to push changes just fine.

bjorn · 2022-01-26T11:37:26Z

Okay there we go, for some reason the setting isn't visible if the repo is in an organization. Moved it to my account and now you should be able to push changes just fine.

Ah, ok! So, that was what the "On user-owned forks" in the docs was about. That wasn't exactly standing out, heh.

* Rely on sfml 0.16 rather than a git revision * Avoid allocating temporary Path * Take into account margin and spacing * Directly produce a FloatRect, since it is what SFML needs * Renamed VertexMesh to QuadMesh * Use Vertex::with_pos_coords since we always passed Color::WHITE * Removed TileLayer wrapper, since it added no value * Removed handling of gid == 0 in Tilesheet::tile_rect

Also fixes the example a bit, because the map was drawn too small.

bjorn · 2022-01-26T13:59:07Z

Is there any way to make this dependency only install when trying to compile that example?

If there is no way to do this, I would be fine with adding the installation of libsfml-dev to the autobuild, btw (and libcsfml-dev, as long as there is no new release of Rust's sfml package yet).

aleokdev · 2022-01-26T17:46:51Z

This is strange. Build is failing as if it the crate modules were private.
Might it be the rust-cache action, which is giving irrelevant data to the CI?

aleokdev · 2022-01-26T17:59:56Z

Any idea why this could be happening? rust-analyzer complains as well but cargo builds without any issues on my local system.

aleokdev · 2022-01-26T18:40:47Z

Alright, fixed, that should do it for the CI. We now have two workflows; One for the library and one for the SFML example. The SFML example one only should run when there has been some change in the examples/sfml directory.

bjorn

Looks fine to me! I'm not really sure about splitting off the build for the SFML example though, since we'd want to make sure it compiles also when making changes to the library anyway.

cobward · 2022-01-31T10:34:09Z

Thanks for this example, it's been a great boost to get going with tiled quickly. I've just got a minor suggestion to improve usability:

-    let map = Map::parse_file(Path::new(MAP_PATH)).unwrap();
+    let mut path = std::fs::canonicalize(".").expect("current dir does not exist");
+    path.push("assets");
+    while !path.exists() {
+        path.pop();
+        if !path.pop() {
+            println!("ERROR: reached file-system root without finding the assets directory");
+            return
+        }
+        path.push("assets");
+    }
+    path.pop();
+    let map = Map::parse_file(path.join(MAP_PATH)).unwrap();

Something like this to allow the example to be run from any directory within the project. Makes it easier to poke around with the example if (like me) you want to use it to understand how rs-tiled works.

Happy to PR this myself once this is merged if you'd prefer.

bjorn · 2022-01-31T10:56:04Z

Something like this to allow the example to be run from any directory within the project. Makes it easier to poke around with the example if (like me) you want to use it to understand how rs-tiled works.

Thanks for trying out the example @G-0-0, and I'm glad it helped you! Regarding your patch, I'm not sure this approach sets a good example. First of all I'd like to know, how were you running the example? Normally I'd expect people to execute cargo run --example sfml, which should have the correct working directory for the example to run.

I've also taken a look at how Bevy finds assets, and they use the CARGO_MANIFEST_DIR environment variable. See their comment here and the implementation here, which falls back to the executable's path if the environment variable isn't available. This looks a bit cleaner to me, so I'm curious if it would also work for you.

aleokdev · 2022-01-31T11:55:21Z

Thanks for trying out the example @G-0-0, and I'm glad it helped you! Regarding your patch, I'm not sure this approach sets a good example. First of all I'd like to know, how were you running the example? Normally I'd expect people to execute cargo run --example sfml, which should have the correct working directory for the example to run.

Have to agree with @bjorn here. Right now my priority with this PR is to get the lowest amount of boilerplate possible, so adding code like that wouldn't show anything to the end user about the library, and furthermore, I expect this code to be run with cargo. As such, I don't think it's appropriate to use it. However, I could replace the unwrap in parse_file with an expect giving the example runner a bit more feedback about why it couldn't load.

cobward · 2022-01-31T12:39:11Z

Thanks both for the responses.

Normally I'd expect people to execute cargo run --example sfml, which should have the correct working directory for the example to run.

I am running this with cargo:

$ cd examples/sfml/
$ cargo r --example sfml
    Finished dev [unoptimized + debuginfo] target(s) in 0.01s
     Running `<redacted>/rs-tiled/target/debug/examples/sfml`
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Other("Map file not found: \"assets/tiled_base64_external.tmx\"")', examples/sfml/main.rs:92:52
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Anyway, my intention wasn't to stop this getting merged, so please go ahead and I can address this afterwards. Also I wasn't aware of that method of finding the executable path, so thanks for sharing the Bevy example, it's a lot nicer.

bjorn · 2022-01-31T13:04:08Z

I am running this with cargo:

Ah, so, in case you were not aware, you don't need to cd examples/sfml/ to run the example (and personally, I was not aware it worked even if you did!). Anyway, I think we should indeed ensure the example works either way.

aleokdev · 2022-01-31T19:14:59Z

That should fix it.

aleokdev · 2022-01-31T19:20:54Z

A few TODOs for this PR:

Add a prettier map
Fix CI (again)

aleokdev · 2022-02-01T12:23:45Z

Merging is blocked by #135.

examples/main.rs

bjorn

This is looking pretty good now I think. Is there a reason the PR is still marked as Draft?

examples/sfml/mesh.rs

aleokdev · 2022-02-11T08:22:41Z

This is looking pretty good now I think. Is there a reason the PR is still marked as Draft?

I'd like a prettier map to show instead of the one we have now.

aleokdev · 2022-02-11T08:24:03Z

examples/sfml/main.rs

+    let mut mesh = QuadMesh::with_capacity(width * height);
+    for x in 0..width {
+        for y in 0..height {
+            // TODO: `FiniteTileLayer` for getting tiles directly from finite tile layers?


Also: This right here

bjorn · 2022-02-11T08:27:02Z

I'd like a prettier map to show instead of the one we have now.

Alright, but I think that could be better as a separate change. It's alright to reuse one of the ugly example maps for now and improve that later.

Regarding support for infinite maps, I also don't think it's required for now, especially since the API for this is still not finalized, and the example map used is not infinite anyway. Similarly the example currently doesn't support maps using multiple tilesets, but it's something that can be improved upon later as well.

aleokdev · 2022-02-11T08:31:28Z

Alright, merging then.

Add SFML example

06ef39d

Adds a rough demo using SFML for rendering.

bjorn reviewed Jan 26, 2022

View reviewed changes

aleokdev mentioned this pull request Jan 26, 2022

Better / More examples #134

Open

bjorn and others added 5 commits January 26, 2022 12:40

Update contributors list

04fdd43

Apply formatting

45ab241

Avoid the need to inverse the camera matrix

dd0ce9a

De-hardcode the tile size

ae64261

Also fixes the example a bit, because the map was drawn too small.

Update workflow

0b1b61f

Try fixing CI issue by removing rust-cache

c185442

aleokdev added the help wanted label Jan 26, 2022

aleokdev added 2 commits January 26, 2022 19:17

Merge remote-tracking branch 'upstream/master' into sfml-ex

3863f9b

Fix CI

bff5abf

aleokdev force-pushed the sfml-ex branch from d6099b4 to bff5abf Compare January 26, 2022 18:40

bjorn previously approved these changes Jan 26, 2022

View reviewed changes

Fix map asset path

0542903

aleokdev dismissed bjorn’s stale review via 0542903 January 31, 2022 19:14

Merge SFML and Rust workflow

70ba76c

Misc changes

a5927ec

aleokdev added 2 commits February 10, 2022 15:47

Merge upstream/master

49fd31f

Update example to master

5300886

aleokdev commented Feb 10, 2022

View reviewed changes

examples/main.rs Outdated Show resolved Hide resolved

aleokdev added 2 commits February 10, 2022 17:13

Merge remote-tracking branch 'upstream/master' into sfml-ex

acb6719

Fix line endings

bcf3002

bjorn previously approved these changes Feb 11, 2022

View reviewed changes

examples/sfml/mesh.rs Outdated Show resolved Hide resolved

Fixed comment

f7c1c03

bjorn dismissed their stale review via f7c1c03 February 11, 2022 08:20

bjorn marked this pull request as ready for review February 11, 2022 08:20

bjorn approved these changes Feb 11, 2022

View reviewed changes

aleokdev commented Feb 11, 2022

View reviewed changes

aleokdev merged commit 187f1fb into mapeditor:master Feb 11, 2022

aleokdev deleted the sfml-ex branch February 11, 2022 08:31

aleokdev added this to the 0.10.0 milestone Mar 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add SFML example #133

Add SFML example #133

aleokdev commented Jan 25, 2022

bjorn left a comment

bjorn commented Jan 26, 2022

aleokdev commented Jan 26, 2022

bjorn commented Jan 26, 2022

aleokdev commented Jan 26, 2022

bjorn commented Jan 26, 2022

bjorn commented Jan 26, 2022

aleokdev commented Jan 26, 2022

bjorn commented Jan 26, 2022

aleokdev commented Jan 26, 2022

bjorn commented Jan 26, 2022

bjorn commented Jan 26, 2022 •

edited

Loading

aleokdev commented Jan 26, 2022

aleokdev commented Jan 26, 2022

aleokdev commented Jan 26, 2022

bjorn left a comment

cobward commented Jan 31, 2022 •

edited

Loading

bjorn commented Jan 31, 2022

aleokdev commented Jan 31, 2022

cobward commented Jan 31, 2022

bjorn commented Jan 31, 2022 •

edited

Loading

aleokdev commented Jan 31, 2022

aleokdev commented Jan 31, 2022

aleokdev commented Feb 1, 2022 •

edited

Loading

bjorn left a comment

aleokdev commented Feb 11, 2022

aleokdev Feb 11, 2022

bjorn commented Feb 11, 2022

aleokdev commented Feb 11, 2022

Add SFML example #133

Add SFML example #133

Conversation

aleokdev commented Jan 25, 2022

bjorn left a comment

Choose a reason for hiding this comment

bjorn commented Jan 26, 2022

aleokdev commented Jan 26, 2022

bjorn commented Jan 26, 2022

aleokdev commented Jan 26, 2022

bjorn commented Jan 26, 2022

bjorn commented Jan 26, 2022

aleokdev commented Jan 26, 2022

bjorn commented Jan 26, 2022

aleokdev commented Jan 26, 2022

bjorn commented Jan 26, 2022

bjorn commented Jan 26, 2022 • edited Loading

aleokdev commented Jan 26, 2022

aleokdev commented Jan 26, 2022

aleokdev commented Jan 26, 2022

bjorn left a comment

Choose a reason for hiding this comment

cobward commented Jan 31, 2022 • edited Loading

bjorn commented Jan 31, 2022

aleokdev commented Jan 31, 2022

cobward commented Jan 31, 2022

bjorn commented Jan 31, 2022 • edited Loading

aleokdev commented Jan 31, 2022

aleokdev commented Jan 31, 2022

aleokdev commented Feb 1, 2022 • edited Loading

bjorn left a comment

Choose a reason for hiding this comment

aleokdev commented Feb 11, 2022

aleokdev Feb 11, 2022

Choose a reason for hiding this comment

bjorn commented Feb 11, 2022

aleokdev commented Feb 11, 2022

bjorn commented Jan 26, 2022 •

edited

Loading

cobward commented Jan 31, 2022 •

edited

Loading

bjorn commented Jan 31, 2022 •

edited

Loading

aleokdev commented Feb 1, 2022 •

edited

Loading