Brookke / SEPR-Assessment2

1 stars 6 forks source link

Joe ben movement #51

Closed joeShuff closed 7 years ago

joeShuff commented 7 years ago

closes #47 closes #48

Brookke commented 7 years ago

Current issue with the game if you press a movement key between ticks the player doesn't move. I assume this is because you are polling the moment not using event based

joeShuff commented 7 years ago

@Brookke That issue is because we have the ticks being once a second, when we have the tweening I will be able to change it to 20 ticks a second and it will be fully responsive

Brookke commented 7 years ago

There is still a chance that it wont work though? Also when we switch screens you will not longer need to need be calling the player.movementTick();

joeShuff commented 7 years ago

When we switch screens? Ticks will run 20 times a second, which is more than enough time for the press to be registered, but as it moves the player a whole tile per tick, we set it to 1 so we can see the movement happening.

Brookke commented 7 years ago

hmm okay I just feel it could lead to bugs and I am not sure how elegant it is. Yes so when we are in the main menu or pause screen we don't need to be moving the player

joeShuff commented 7 years ago

Yeah the changing of screens will be dealt with later. Right now we need the code to be working. We can easily move it and add parameters as to whether it runs or not. What bugs do you think it will run in to?

jasonmash commented 7 years ago

I think I agree with @brookke on this one, using the loop with thread.sleep doesn't seem very elegant. I can see this approach will work, but I'd be more comfortable with the code knowing it was all event-driven instead.

Also just run a test to get the FPS on my local machine between this branch and dev - there is a considerable drop in this branch. Like in dev I get between 350-450 fps, in this branch I get 150-250fps. It's not a big deal at the moment, but if we're definitely taking this loop approach can we be sure that it won't have too much of a performance impact, especially with the view that we will be expanding the codebase significantly over the next few months.

joeShuff commented 7 years ago

I get what you're all going on about but this game isn't solely event driven... At all. We want NPCs, that in its own is purely not event driven...

joeShuff commented 7 years ago

Also @jasonmash the reason for the drop is because we are now running a game loop. Games have loops, they have to, its what makes them games and not just a piece of software. We don't ned 350-400 FPS in a tiled game, we can get away with like 20-30 as its not important.

Brookke commented 7 years ago

I thought libgdx did the loop itself is all. But also we should aim for 60 fps on our computers as we want to make sure it runs well on worse case hardware

joeShuff commented 7 years ago

The LibGDX loop as far as I know is for rendering only.

joeShuff commented 7 years ago

As we don't have access to that loop. Other than putting all the methods in the render method. But I'll take a look

joeShuff commented 7 years ago

Also with building our own loop, we can control it, whereas LibGDX might just run it as often as they possibly can

Brookke commented 7 years ago

http://gamedev.stackexchange.com/questions/71883/game-update-in-libgdx Read the answer to this it's quite good

joeShuff commented 7 years ago

As I said, we'd be using the render method, which in my opinion is messier... using the render() method to do something completely different to rendering...

Brookke commented 7 years ago

the render method is to do the game logic before rendering the game? And also most events should be event driven

joeShuff commented 7 years ago

Like it says. LibGDX is purely event driven. That's why they don't have an update method. The render method is for rendering to the screen, I think it's messier to put something other than rendering into that method. What about having npcs strolling around? They will need to be controlled. While some of the aspects of the game are event driven, its not all event driven. Also... Your continuous event driven you want, that itself is not event driven. There is an event to get the keys up and down, everything else is not event driven.

jasonmash commented 7 years ago

Which parts of the game aren't event driven? Keep in mind I'm talking about the bare minimum for the game here, not extra things we could do like NPCs walking around freely.

Also, correct me if I'm wrong, surely the render method is responsible for generating everything to put on screen? for example, wouldn't the render method would need to run some code to find out where an NPC is, in order to render it?

joeShuff commented 7 years ago

The way we've had to do continuous is using a loop. Cause an event happens when a key goes down, and an event triggers when its lifted up, nothing in between... Hence that needs a loop to work

joeShuff commented 7 years ago

If you want to read the render() method as to do all the logic aswell feel free. But its called render() because that is all it does. In some bigger game engines there are more methods. update() being one of them, and you can set how often the game calls all the objects update() method. It's exactly what we're doing here

jasonmash commented 7 years ago

Ah okay, I understand what you mean now.

How do other developers do sprite movement in libgdx? I'm sure other developers have had to do the same thing before. I'm sure your way will work, I just wonder if there's a more efficient way of implementing it without a global game timer.

Brookke commented 7 years ago

@jasonmash it seems they use the render method, hmm we should discuss tomorrow. The current way seems to be very inefficient and not really in the life cycle of libgdx Render(): Method called by the game loop from the application every time rendering should be performed. Game logic updates are usually also performed in this method. https://github.com/libgdx/libgdx/wiki/The-life-cycle

joeShuff commented 7 years ago

The problem with using the render() method is we can't control its frequency...

Brookke commented 7 years ago

http://stackoverflow.com/questions/14247613/libgdx-game-logic-in-render this is when you use delta

Brookke commented 7 years ago

And you can limit the fps to 60 for the game or even lower if we needed too

joeShuff commented 7 years ago

Delta being the time since the last frame... How can we use that to set a TPS?... Slow computers delta will be bigger. Fast computers it will be smaller...

Brookke commented 7 years ago

so all your units are in units/seconds then you update them based on delta e.g. if the person has been holding the left key 2 seconds they could move 10 tiles therefore the units per delta would be 5 tiles/second

Brookke commented 7 years ago

and its always rounded up to the nearest tile

joeShuff commented 7 years ago

Can we go through this tomorrow? Seems like quite a difficult concept to grasp

jasonmash commented 7 years ago

Before we spend too much time debating this feature, remember it's nice to have, rather than essential. So unless we get a solution that we agree on soon (and I hope we do), I'd suggest pausing this pull request/issue until we've done some more of the essential parts of the game. We can always come back to this discussion if we run into a need for more loops in the future. Is that alright?

joeShuff commented 7 years ago

As much as I agree with that, I think the issue is how we're going to do the loops rather than having them completely. Me and Ben were told to implement continuous movement. We need loops.

Brookke commented 7 years ago

Using the Brooke-movement branch instead