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

Rendering #1

Merged
merged 16 commits into from
Apr 20, 2024
Merged

Rendering #1

merged 16 commits into from
Apr 20, 2024

Conversation

Challmymind
Copy link
Contributor

Creating draft to discuss rendering development here.


/// SDL unit
pub struct UnitSDL<'a> {
pub position: &'a (i32, i32),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we really need a reference to 2 4-byte numbers? :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

[NOT OUTDATED] It's not about memory it's about having reference to existing object so if we update it's location then the rendering will also be updated. My pipeline caches objects to be rendered and it is workaround to make updates easier. But there are still issues with it:

  1. What happens when original entity is destroyed? Because of explicit lifetimes all clusters using this object are destroyed -> in the case when compiler don't find issue, we have runtime error
  2. Also i32? If we want to pass screen's coordinates and not game's coordinates then it's meaningless because screen can have only positive numbers in SDL -> we need to decide where translation to the correct coordinates happens

type Unit<'a>;
type Render<'a>: Render<Unit<'a> = Self::Unit<'a>>;

/// Creates Renderer.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not a RenderBuilder?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

[NOT OUTDATED] Comments must be updated -> i will link to the resolving commit (should it happen :D )

pub struct UnitSDL<'a> {
pub position: &'a (i32, i32),
pub texture: Option<&'a sdl2::render::Texture<'a>>,
pub color: Option<sdl2::pixels::Color>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need this? As an alternative to texture?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For the record i will write here my point that i made during our talk:
Yes, because it's fallback for us to make sure we aren't forced to use textures when we don't need to.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I forgot how powerful are enums in rust, it will be swapped to the enum -> I will link the resolving commit here.

@Challmymind Challmymind self-assigned this Apr 6, 2024
@Challmymind
Copy link
Contributor Author

Create render_map() <- to render hexagons

@Challmymind Challmymind marked this pull request as ready for review April 14, 2024 17:16
@@ -6,7 +6,7 @@ edition = "2021"
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
sdl2 = { version = "0.36.0", features = ["bundled"]}
sdl2 = { version = "0.36.0", features = ["bundled", "gfx", "image"]}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Te nowe biblioteki mogą wymagać gfx i image na systemie, tak? Może w takim razie przydałoby się jakieś info w README?

input::Input,
simulation::{simulation::Simulation, world_state::WorldState},
};

use super::logging::init_logging;

pub struct Game<D: GameDisplay> {
display: D,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please don't, this trait here is for a reason

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was quick fix to make sure examples are compiling because we had name clash with my/your display mod. I'll resolve it

@stabor705 stabor705 merged commit ba4e067 into master Apr 20, 2024
6 of 8 checks passed
@HikariIT HikariIT deleted the rendering branch May 10, 2024 16:17
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.

2 participants