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

updates for window events #453

Draft
wants to merge 1 commit into
base: v0.5
Choose a base branch
from
Draft

updates for window events #453

wants to merge 1 commit into from

Conversation

Sygmei
Copy link
Member

@Sygmei Sygmei commented May 8, 2023

No description provided.

@@ -193,6 +193,14 @@ namespace obe::system
std::string title = "ObEngine";
if (configuration.contains("title"))
m_title = configuration.at("title");

event::EventNamespace event_namespace = event::EventNamespace("obe::system");
Copy link
Member Author

Choose a reason for hiding this comment

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

Check out :

m_event_namespace = &m_events->create_namespace("Event");

for how to create a EventNamespace, although that made me realize that the constructor should be private, I'll fix this :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok I see this, do let me know when you have your changes in place for the constructor.

Copy link
Member Author

Choose a reason for hiding this comment

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

Should be good, rebase your branch on latest v0.5 commit :)

struct Moved
{
static constexpr std::string_view id = "Moved";
Position new_pos;
Copy link
Member Author

Choose a reason for hiding this comment

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

As in

A position and previous_position naming scheme would be more appropriate imo

Copy link
Collaborator

Choose a reason for hiding this comment

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

@symei I have a Position new_pos and then Position previous_pos, Position is a struct that has unsigned int values for x,y. Are you saying remove the Position struct, and just have unsigned int x,y values vs a struct called Position, that contains x,y?

Copy link
Member Author

Choose a reason for hiding this comment

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

You can keep the Position struct, just rename new_pos to position and previous_pos to previous_position and we're good to go :)

@@ -202,6 +210,10 @@ namespace obe::system
m_window.setKeyRepeatEnabled(false);

this->apply_view();
events::Window::Size new_size { m_width, m_height };
events::Window::Size previous_size { 0, 0 };
e_window->trigger(
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure we should trigger Resized here, eventually, we could add a Created or Opened event for this

@Sygmei
Copy link
Member Author

Sygmei commented May 8, 2023

The general structure looks good, I added a few comments :)

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