UnderwaterApps / overlap2d-runtime-libgdx

Overlap2D - UI and Level Editor libgdx runtime
http://overlap2d.com
Other
170 stars 96 forks source link

Rotation and scaling issue fixed #69

Closed simoarpe closed 8 years ago

simoarpe commented 8 years ago

All the changes should be 100% compatible with the previous logic, just added more options: passing your own world, using custom physics, using custom light.

azakhary commented 8 years ago

This looks legit, but there are several points. 1) Not sure why we need custom gravity boolean. You can always set different gravity after scene has loaded? is there set gravity method in box2d? or it has to be set in constructor? (also, instead of custom gravity boolean maybe just pass the gravity vector instead?

2) I just noticed we have some physics stuff going on in renderer, this has to be moved to the PhysicsSystem maybe?

I'll check this branch out tomorrow, see if I can do that. But till then, we can discuss.

simoarpe commented 8 years ago

1) Yes there is setGravity in box2D, but for my case, I needed a morphing gravity depending on which platform the player is onto. This was a requirement just for some scene, so for not overcomplicating my life I added a new method loadScene(String sceneName, Viewport viewport, boolean customGravity, boolean customLight) in order to start in a "clean state" and setting the gravity continuously using my custom algorithm. For the scene where I needed a "conventional gravity" I used the previous loadScene(String sceneName, Viewport viewport) fully working and uses the gravity set from the Overlap Editor.

2) I agree physics should be moved to the PhysicsSystem, can you point out the line number the render is doing physics stuff?

s.

2015-12-21 17:56 GMT+00:00 Avetis notifications@github.com:

This looks legit, but there are several points. 1) Not sure why we need custom gravity boolean. You can always set different gravity after scene has loaded? is there set gravity method in box2d? or it has to be set in constructor? (also, instead of custom gravity boolean maybe just pass the gravity vector instead?

2) I just noticed we have some physics stuff going on in renderer, this has to be moved to the PhysicsSystem maybe?

I'll check this branch out tomorrow, see if I can do that. But till then, we can discuss.

— Reply to this email directly or view it on GitHub https://github.com/UnderwaterApps/overlap2d-runtime-libgdx/pull/69#issuecomment-166374801 .

SA

azakhary commented 8 years ago

Gor gravity, let's make it work with setter not constructor. even for your case. you can just call setGravity after you loaded that particular scene. (You can add setGravity method to sceneloader that will call appropriate method for world) Does that make sense?

overall looks really good. Let's have this small changes done, make sure it works, and merge!

azakhary commented 8 years ago

Sorry I was super busy, this is not forgotten, will totally test and merge.

simoarpe commented 8 years ago

sorry for the last commit, I know it's something needs to be done but probably in a proper branch and when you think so. I'll revert this to 1.71

olipoh commented 8 years ago

Hi,

and sorry if I missed something (because I'm new to almost everything) or if it's not the right place to comment.

I applied the changes, following this : https://github.com/simoarpe/overlap2d-runtime-libgdx/commit/8bcb0836ac96cf5c744d9890fcec2ac5a7d3d004

Now the rotation problem on assets with physics is fixed, but there is no more collision. I use exactly the same overlap2d scene. I have 2 branches, the only difference is the rotation fix. One branch with the fix : rotation ok, collision fails The other one without the fix : rotation fails, collision ok

As I used the box2d debug renderer, I immediately saw that the bodies' boxes had disappeared. It's a problem with bodies coordinates. If you change these 2 lines of PhysicsBodyLoader class : verts[j] += physicsComponent.centerX; verts[j+1] += physicsComponent.centerY;

in

            verts[j] += physicsComponent.centerX * scale;
            verts[j+1] += physicsComponent.centerY * scale;

Everything turns to work properly.

Does it make sense to you ?

simoarpe commented 8 years ago

hi olipoh, follow these two commits: https://github.com/simoarpe/overlap2d-runtime-libgdx/commit/8bcb0836ac96cf5c744d9890fcec2ac5a7d3d004 (you did it already) https://github.com/simoarpe/overlap2d-runtime-libgdx/commit/2f43f5bf331ac34ba3b5860dcdd9541efe7297f5

2016-01-25 12:30 GMT+00:00 olipoh notifications@github.com:

Hi,

and sorry if I missed something (because I'm new to almost everything) or if it's not the right place to comment.

I applied the changes, following this : simoarpe@8bcb083 https://github.com/simoarpe/overlap2d-runtime-libgdx/commit/8bcb0836ac96cf5c744d9890fcec2ac5a7d3d004

Now the rotation problem on assets with physics is fixed, but there is no more collision. I use exactly the same overlap2d scene. I have 2 branches, the only difference is the rotation fix. One branch with the fix : rotation ok, collision fails The other one without the fix : rotation fails, collision ok

As I used the box2d debug renderer, I immediately saw that the bodies' boxes had disappeared. It's a problem with bodies coordinates. If you change these 2 lines of PhysicsBodyLoader class : verts[j] += physicsComponent.centerX; verts[j+1] += physicsComponent.centerY;

in

        verts[j] += physicsComponent.centerX * scale;
        verts[j+1] += physicsComponent.centerY * scale;

Everything turns to work properly.

Does it make sense to you ?

— Reply to this email directly or view it on GitHub https://github.com/UnderwaterApps/overlap2d-runtime-libgdx/pull/69#issuecomment-174491733 .

SA

olipoh commented 8 years ago

hi simoarpe,

ok, I just followed the second commit, but the issue stays the same : for me, it works only with these two lines verts[j] += physicsComponent.centerX * scale; verts[j+1] += physicsComponent.centerY * scale;

olipoh commented 8 years ago

this makes sens because first, you substract physicsComponent.center from minpolygondata, which is then multiplied by scale. So when at last you add physicsComponent.center again, it should be multiplied by scale too. no ? it's not consistent ?

simoarpe commented 8 years ago

yes it makes sense you're right! I've added back the scale factor and updated the pull request. Thank you.

azakhary commented 8 years ago

Works for me on first test, but it's a big change, I want to be sure before we merge this. Could you guys test this too?

azakhary commented 8 years ago

(Specially with the editor)

olipoh commented 8 years ago

Alright, what do you want me to test ? I never thought the editor was involved...

azakhary commented 8 years ago

Well the editor uses this particular repo in order to do it's own rendering. Testing is - running editor from sources, when you do it will pull this repo and, it should be tested if things behave normal, rotating scaling them e.g.

azakhary commented 8 years ago

I'll do most of the testing, but if you guys can also double-check that would be more bullet proof, as you know what is changed better.

simoarpe commented 8 years ago

Glad to help. I'll do some tests too

2016-01-25 17:36 GMT+00:00 Avetis notifications@github.com:

I'll do most of the testing, but if you guys can also double-check that would be more bullet proof, as you know what is changed better.

— Reply to this email directly or view it on GitHub https://github.com/UnderwaterApps/overlap2d-runtime-libgdx/pull/69#issuecomment-174597129 .

SA

olipoh commented 8 years ago

I would be glad to help too, but as I told, I'm new to almost everything. Today was the first time I picked the sources and I'm still surprised I could use and modify them. I'm not sure I understood everything you told but I want to learn. So what should I do ? Download the editor sources and take care not to overwrite what was changed by the commit ? Sorry if I look newbie, but actually I am :-).

olipoh commented 8 years ago

Or maybe I should open the editor as a new project in android studio and apply the above changes ?

azakhary commented 8 years ago

Here is what you do. first of all, ignore the folder you were working on previously, so you will not overwrite it. Do all in somewhere else. fork overlap2d editor sources, and then in some other place on your local, "clone your fork" then use your IDE to import it a gradle project, do gradle refresh and build, this will automatically download latest runtime in editor folder. And then you should be able to run the editor. If all runs good that means you are running editor with latest runtime sources. Now what you need to do is fetch your "changes" into that folder, in order to see if they work. Doing that is simple, just enter the runtime folder (which is in editor folder) and run:

git checkout -b simoarpe-master master git pull https://github.com/simoarpe/overlap2d-runtime-libgdx.git master

this will create a test branch and fetch your changes into it. Then run editor again, and try bunch of things, make sure all works as previously. If so - all is good.

olipoh commented 8 years ago

I think I understood everything (meaning it's well explained :-)). I'm not I'll have time tomorrow, but after tomorrow, I should have time. Is that ok ? Thank you.

azakhary commented 8 years ago

Yup it's totally ok. I'll check it earlier, then you guys give your test info too. Then we merge.

olipoh commented 8 years ago

Hi...

... and ouch !!! How I suffered :-). I even don't know if I did everything right... I had to install java jdk8 (as I only had jre8 and jdk7). I had to create a run conf, which was not obvious to me, and finally had to set the working directory to overlap2d>assets. But finally it worked, thumbs up.

Now, the fetch part. You spoke about a runtime folder in the editor folder. But I didn't find it. There is a overlap2d folder (editor ?), a overlap2d-runtime-libgdx folder (runtime ?), and some others. So I supposed you were talking about the second one. I pulled simone's changes there. But...

but I have an error : Information:Using javac 1.8.0_72 to compile java sources Information:java: Errors occurred while compiling module 'overlap2d' Information:26/01/16 07:35 - Compilation completed with 1 error and 0 warnings in 1s 529ms /home/olivier/workspace/libgdx/overlap2d/overlap2d-runtime-libgdx/src/com/uwsoft/editor/renderer/SceneLoader.java Information:Information:java: /home/olivier/workspace/libgdx/overlap2d/overlap2d-runtime-libgdx/src/com/uwsoft/editor/renderer/SceneLoader.java uses unchecked or unsafe operations. Information:Information:java: Recompile with -Xlint:unchecked for details. /home/olivier/workspace/libgdx/overlap2d/overlap2d/src/com/uwsoft/editor/view/stage/Sandbox.java Error:Error:line (142)java: cannot find symbol symbol: method setPhysicsOn(boolean) location: class com.uwsoft.editor.renderer.systems.render.Overlap2dRenderer

So : master branch --> overlap2d runs without any problem simone branch --> error described above

In fact, I did what you told about fetching and I'm a bit surprised. I was expecting 2 branches in one local repository but actually I have 3 repositories (I didn't create them by myself):

master and simoarpe-master and master are in the second one... I'm a bit confused.

azakhary commented 8 years ago

Yep it normal to have 3 repositories. There is no other way to do it. now when you pull the runtime you should be pulling it for the runtime repository. If it's command line, you should go into that folder: cd overlap2d-runtime-libgdx and then do the pull, so it pulls for that repo.

As for the error it's pretty weird. Is there any chance you pulled the runtime sources into the editor repo or something similar?

azakhary commented 8 years ago

There should be difference, the one is latest repo, the other is your chances. Maybe official repo changed a bit after you did your changes or something?

olipoh commented 8 years ago

It's a bit chinese for me... I didn't pull the runtime sources by myself, android studio / gradle did it. In overlap2d root directory, I have sub-directories:

As for the official repo, what do you call official repo ? I hope I didn't change anything. I didn't write any code, didn't commit nor push at all. I just followed your instructions and tried to run...

olipoh commented 8 years ago

I checked on github:

UnderwaterApps/overlap2d-runtime-libgdx : setPhysicsOn is present

simoarpe/overlap2d-runtime-libgdx : setPhysicsOn is not

in com.uwsoft.editor.renderer.systems.render.Overlap2dRenderer

olipoh commented 8 years ago

If it helps, here is a copy of my terminal when I pulled:

olivier@pluton:~/workspace/libgdx/overlap2d$ cd overlap2d-runtime-libgdx olivier@pluton:~/workspace/libgdx/overlap2d/overlap2d-runtime-libgdx$ git checkout -b simoarpe-master master Basculement sur la nouvelle branche 'simoarpe-master' olivier@pluton:~/workspace/libgdx/overlap2d/overlap2d-runtime-libgdx$ git pull https://github.com/simoarpe/overlap2d-runtime-libgdx.git master remote: Counting objects: 120, done. remote: Compressing objects: 100% (51/51), done. remote: Total 120 (delta 52), reused 107 (delta 39), pack-reused 0 Réception d'objets: 100% (120/120), 11.10 KiB | 0 bytes/s, fait. Résolution des deltas: 100% (52/52), complété avec 23 objets locaux. Depuis https://github.com/simoarpe/overlap2d-runtime-libgdx

azakhary commented 8 years ago

This seems to be correct. It should be working, and the physics should not be related. That's weird.

olipoh commented 8 years ago

As I told, Overlap2dRenderer.java is different on UnderwaterApps/overlap2d-runtime-libgdx and simoarpe/overlap2d-runtime-libgdx. Is that normal ?

I created a new branch locally and replaced the Overlap2dRenderer.java. Now it runs but when loading a recent project, the screen stays black... does this help ?

olipoh commented 8 years ago

In fact I don't understand when you say that physics should not be related... Overlap2drenderer should not have changed or the changes should not lead to that error ?

(calling a method which does not exist should lead to an error I suppose)

If you do the same, it works for you ?

azakhary commented 8 years ago

changes should not lead to an error. And yes, why should it NOT be different? You made some changes on your fork. Your changes are not yet merged. Thus, your changes are NOT on the official repo, so official repo is different from your changes.

official repo -> UnderwaterApps/overlap2d-runtime-libgdx your changes -> simoarpe/overlap2d-runtime-libgdx

They are different because your code is not changed. That is why you fetch your changes by pulling them in order to test them.

olipoh commented 8 years ago

But it's not my changes. I'm not Simone.

I just forked underwater project, then pulled simone changes, but myself didn't change anything (just to be clear).

And to be sure, I tried something else. I created another branch from master (the official repo right ?), then I applied those 2 commits by hand: simoarpe@8bcb083

simoarpe@2f43f5b

and it works (well, at least the editor runs).

I don't know if it's because of my english because I'm too newbie but it's difficult for me. Yesterday I was excited, now I have serious doubts on my capacities...

azakhary commented 8 years ago

Aw, I get it, sorry.

Well, I was thinking that because we are testing "this pull request" and this pull request is simonarpe, that's why in order to test it, it's codes has to be retrieved.

olipoh commented 8 years ago

So, what does it mean ? What should I do now ?

azakhary commented 8 years ago

You did all things right, but it was crashing for some reason, I cannot really tell why. But I can test things myself, and them merge or not merge this pull request. You can try to figure out why it crashes, but it might me redundant at this point.

simoarpe commented 8 years ago

My apology for not having tested against the editor, I didn't know it was sharing the same codebase. I've fixed what was crashing it. Tested with my scenes (some of them are quite complex), and it seems to run smoothly. setPhysicsOn in the Render class is totally useless now because the physics is totally decoupled from the render. Anyway I've left the method in order to avoid any compatibility issue/crash.

olipoh commented 8 years ago

So does it mean I should pull your repo again ?

simoarpe commented 8 years ago

You are more than welcome to try and check, but it's up to you

2016-01-26 12:14 GMT+00:00 olipoh notifications@github.com:

So does it mean I should pull your repo again ?

— Reply to this email directly or view it on GitHub https://github.com/UnderwaterApps/overlap2d-runtime-libgdx/pull/69#issuecomment-174976823 .

SA

olipoh commented 8 years ago

Ok, it doesn't crash anymore. But I have a real problem that I absolutely don't understand. Yesterday, everything was ok (at least with predictable results), with your fix and without, now I have a surprising result. And it is the same in all my configurations:

all combined as you can imagine. So this means that some configurations, same as yesterday give different results. Of course there is an explanation but I can't figure it out.

capture du 2016-01-26 13 58 02

As you can see, there is a shift between the bitmap and the body... PS : I don't forgot to export from the editor (I even changed the scene slightly to be sure it was done).

azakhary commented 8 years ago

Looks like origin/rotation thing?

olipoh commented 8 years ago

Yes but why ? I didn't change anything, if I would, I would have done it in one place, not in all the branches and repos I'm testing. I have the same shifting everywhere... the only thing that changed globally is that my jdk is now 8. I even downgraded to 7, but I have the same strange result... if you had a scene of yours, I could test it and see if this is the problem... I really feel powerless. There must be a good and easy answer, but I don't see...

azakhary commented 8 years ago

Does this happen when editor is ran from sources or from the jar that is on the website? If it's sources: 1) which branch are you on - editor repo 2) which branch are you on - runtime repo

simoarpe commented 8 years ago

very odd, mine looks fine [image: Immagine incorporata 1]

2016-01-26 13:33 GMT+00:00 Avetis notifications@github.com:

Does this happen when editor is ran from sources or from the jar that is on the website? If it's sources: 1) which branch are you on - editor repo 2) which branch are you on - runtime repo

— Reply to this email directly or view it on GitHub https://github.com/UnderwaterApps/overlap2d-runtime-libgdx/pull/69#issuecomment-175013328 .

SA

olipoh commented 8 years ago

I can run editor from jar or from sources, it's same result...

And to test the result in my "game", I use another project. I run it with overlap2d runtime, or from sources in simoarpe-master branch. The strange thing is that I have the same "shift" that I use runtime in jar or from sources... Maybe I should let it sleep for one day and come back with a clear and fresh mind :-).

olipoh commented 8 years ago

Even if it seems very unlikely, I'm wondering if my computer could be the reason. I had another issue with overlap2d that it seems that I am the only one to experience. Here is the discussion : http://overlap2d.com/forums/topic/strange-behavior-with-physics/ It's related to physics too. I'm using ubuntu 14.04 64bits but I don't see why it would responsible for my troubles.

olipoh commented 8 years ago

Would it be possible that there is a new box2d version, that has been refreshed when I resynced gradle ? And that I have a problem with box2d ?

azakhary commented 8 years ago

Try clearing your gradle cache

olipoh commented 8 years ago

Didn't change anything.

I will let the night pass, maybe tomorrow I'll find the light. Thank you very much for your help. If you have time to watch this discussion (just the videos I posted) : http://overlap2d.com/forums/topic/strange-behavior-with-physics/ and then tell me where to investigate, it would be nice.

olipoh commented 8 years ago

Hi,

so... I decided to create a new scene (with 10 pixels per unit). And I recreated my local clone repo. Then I applied the simoarpe's correction. And everything works fine, at least visually. My first basic test is ok. I now should be able to test a little bit more.

olipoh commented 8 years ago

When I launch overlap2d from sources, I have a warning message, appearing twice. It happens with or without simone's correction: VisUI: Warning, using default favorites preference name for file chooser! (see FileChooser.setFavoritesPrefsName(String)) I have twice this message.

And when I export the scene (Ctrl-e), I have an error message. This message appears twice, with or without Simone's correction. And the editor doesn't crash. Message attached. error_message.txt