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

[RFC] Prototyping speed improvements #18

Open
AnneKitsune opened this issue Feb 26, 2018 · 31 comments
Open

[RFC] Prototyping speed improvements #18

AnneKitsune opened this issue Feb 26, 2018 · 31 comments

Comments

@AnneKitsune
Copy link
Contributor

Problem

Prototyping in amethyst is slow.

100 lines of code seems to be the minimum to make the smallest of game. While its not a lot, it definitely is more than necessary.

Source

I'm taking https://github.com/amethyst/amethyst/blob/develop/examples/sphere/main.rs as a model
Lack of good defaults for generic types.
Imports
Lack of handle_event utils for simple cases -> Quit on any key, quit on some key, is key pressed.

Propositions

Lack of good defaults for generic types.

Adding sensible defaults as type alias for generics.

  1. Add a default.rs file.
  2. Make a DefaultSomething type alias with a default consistent with the others.
  3. Export the module default.

Example:
amethyst_input/src/default.rs

type DefaultInputHandler = InputHandler<String,String>

Imports

Expand the prelude to include as many common types as possible. I'm not too sure how much this slows down compile time when using the prelude, but I do have performance degradation in one of my project where I was using only preludes.

Lack of handle_event utils for simple cases -> Quit on any key, quit on some key, is key pressed.

I already implemented it inside of a custom project, currently debating if this should be inside of the engine.

@AnneKitsune
Copy link
Contributor Author

first!

@majstudio
Copy link

Bump !

@Rhuagh
Copy link
Member

Rhuagh commented Feb 26, 2018

I'm fine with this, but please don't use external libraries which are unreleased (and not even close to complete) as the main example.

@Rhuagh Rhuagh closed this as completed Feb 26, 2018
@Rhuagh Rhuagh reopened this Feb 26, 2018
@Rhuagh
Copy link
Member

Rhuagh commented Feb 26, 2018

Oops, wrong button.

@Rhuagh
Copy link
Member

Rhuagh commented Feb 26, 2018

I mean, when is a normal user going to use something else, except in an extremely low amount of edge cases?

Just a personal opinion on this, but I would never use String.

@AnneKitsune
Copy link
Contributor Author

Not even for your prototypes?

@Rhuagh
Copy link
Member

Rhuagh commented Feb 26, 2018

Not even for my prototypes.

@AnneKitsune AnneKitsune changed the title [RFC] Defaults [RFC] Prototyping speed improvements May 2, 2018
@torkleyy
Copy link
Member

MaterialDefaults is a resource, I think it should be a constant.

No, it should not. It's a fallback for when a texture handle is invalid and the user may provide it. Also, a constant would need late initialization (and yeah involve global state, probably not work with multithreaded tests, ..).

@AnneKitsune
Copy link
Contributor Author

Lack of handle_event utils for simple cases -> Quit on any key, quit on some key, is key pressed.
I already implemented it inside of a custom project, currently debating if this should be inside of the engine.

Thinking of adding it in amethyst_input/src/utils.rs instead of amethyst_utils.
Would that make sense @torkleyy ?

ps: I removed the material defaults part.

@Rhuagh
Copy link
Member

Rhuagh commented May 19, 2018

Sounds like an excellent location.

@torkleyy
Copy link
Member

Hmm I'm not sure, creating a convenience function for everything might obscure things too much.

@AnneKitsune
Copy link
Contributor Author

AnneKitsune commented May 19, 2018

So, who wants to have this: https://github.com/jojolepro/amethyst-extra/blob/master/src/lib.rs#L193 (lines 193 to 257 included into the engine, and who doesn't? I don't mind keeping it external, but integrating it would make the examples cleaner.

@Xaeroxe
Copy link
Member

Xaeroxe commented May 19, 2018

I agree there shouldn't be a function for everything, but this qualifies as extremely common. I say we should add it.

@MrMinimal
Copy link

Generating a quad and handling basic events should be part of the engine, I'd agree it's okay to add it. As long as we keep @torkleyy's concerns in mind for other (less integral) features.

@torkleyy
Copy link
Member

torkleyy commented May 19, 2018

Well, I think it would be nice to have a more concise Event type. It should just be

match event {
    Event::KeyPressed(KeyCode::Escapce) => blah,
}

That would make things a lot nicer.

@torkleyy
Copy link
Member

That's why I'm still in favor of exposing our own event type instead of winit::Event.

@Xaeroxe
Copy link
Member

Xaeroxe commented May 19, 2018

Why not expose both?

@torkleyy
Copy link
Member

Exposing just our own type allows us to upgrade winit without breakage. Additionally, if we convert winit events into our own type, I don't see any need to interface with the winit version.

@torkleyy
Copy link
Member

torkleyy commented May 19, 2018

Having two ways of dealing with the problem would be confusing IMO.

@Xaeroxe
Copy link
Member

Xaeroxe commented May 19, 2018

That's fair. I was mostly thinking of people who might want to use winit events to interface with other winit compatible crates, but I'm not sure I have any good examples at this time.

@Rhuagh
Copy link
Member

Rhuagh commented May 19, 2018

I do that, I convert winit events myself into other events.

@Rhuagh
Copy link
Member

Rhuagh commented May 19, 2018

I do think we can add a system event for the things that are hard events

@torkleyy
Copy link
Member

for the things that are hard events

What do you understand by that?

@Rhuagh
Copy link
Member

Rhuagh commented May 19, 2018

Window close, resize etc, things that you usually don't rebind.

@torkleyy
Copy link
Member

I'm not sure I understand what you're suggesting, sorry.

@Rhuagh
Copy link
Member

Rhuagh commented May 19, 2018

I'm not even sure I do myself :P

My current gripe with the current handle_event function calls are that there's absolutely no control from the users perspective what events get propagated there. For the most part if you want input in a usable manner today you'd request access to an EventChannel manually, and just ignore what comes to handle_event. Somehow I guess to basic use case for it was the ability to get easy access to input events.

The problem however is that input events can roughly be divided into two groups of events:

  • What I call hard events - Close, Resize, Focus etc - Things that are never rebound, just acted upon
  • Rebindable events - Mouse, Keyboard, Touchpad etc, i.e. actual input events, which are most likely passed through some kind of rebinding and transformed into an event structure specific to each game.

I'm not sure I even have a proposal right now, just some gripes.

@raskyld
Copy link

raskyld commented Oct 30, 2018

Is this issue up-to-date ?
Are all listed problems still there?

@torkleyy
Copy link
Member

@AlmightyScientist Yes, this issue did not change a lot. I'll go through all the RFCs soon and make sure they're all discussed properly.

@raskyld
Copy link

raskyld commented Nov 2, 2018

Should we use the <T=ConcreteType> notation or use a typedef (type Default... = ...<ConcreteType>) ?

@AnneKitsune
Copy link
Contributor Author

<T=ConcreteType> where it makes sense to have a default type. For example, draw Passes shouldn't have a default for the data because there's no good default (PosTex, PosNormTex, PosNormTangTex, etc)

Those that don't should have a type Default..

@fhaynes
Copy link
Member

fhaynes commented Jan 8, 2019

Transferring to RFC repo

@fhaynes fhaynes transferred this issue from amethyst/amethyst Jan 8, 2019
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

No branches or pull requests

8 participants