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

Lighting #1881

Merged
merged 57 commits into from Aug 17, 2020
Merged

Lighting #1881

merged 57 commits into from Aug 17, 2020

Conversation

HarsimranVirk
Copy link
Contributor

@HarsimranVirk HarsimranVirk commented Jul 20, 2020

Alright, @4ian I have got this branch running on top of master (instead of pixi-v5). This is cherry-picked from the previous branch with some changes related to LayersList. Also, notifying @NilayMajorwar about a new command I've added in UseLayersListCommands which uses the LightingLayerDialog to edit properties of lighting layer :)

(Also resolves #1883)

… is created. Added DismissableAlertMessage in LightingLayerDialog. Added function in runtimescene.js to get lighting layer.
@Bouh
Copy link
Collaborator

Bouh commented Aug 7, 2020

The sprite is inside another sprite, but the raycast sorting should take care of all corners if the sprite is in the light area.
But it's happend also without collide, but the obstacle is not entirely in the light area.

Can you explain how can I reproduce this? I'm not even sure what the exact problem is, but let's see if we can fix it 😬.

I've added some colours, in blue you have the sprite area, in yellow their corner, in pink two arrows. Two area in purples.
image

You can see the top right corner is inside the light area, this one work fine.

The other corners are not in the light area and the debug mode show us there are two bad lines (pointed with the pink arrows).
So the purple area should be included as well, this is not actually the case because i guess the algorithm doesn't care of the 3 other corners outside of the light area.

@HarsimranVirk
Copy link
Contributor Author

this is not actually the case because i guess the algorithm doesn't care of the 3 other corners outside of the light area.

@Bouh, I guess that's the problem, the algorithm does care about the other 3 corners as well. The "Light Obstacles Manager" either includes the entire object (which would mean all the corners) or it discards the entire object 🤔. This is why you can see the red-colored ray being cast towards the top-left vertex of the sprite object, which hits the boundary of the "light's area", and we get that purple triangle at top of the sprite. Similarly, for the bottom triangle, the red-colored ray is being cast towards the bottom-right vertex 🤔.

@blurymind
Copy link
Contributor

Does/Will this also support normal map textures on sprites?

@blurymind not in this pr I guess, but I'm working on a prototype right now (I might push a draft pr this weekend hopefully 😬). @4ian and I decided that the normal maps rendering would be separate from "raycasting lights". Let's see if we could make it work 🤞.

That is an excellent idea. Some people might want to apply only one or the other, some might want both

@Bouh
Copy link
Collaborator

Bouh commented Aug 7, 2020

I'm thinking about it now but do you also support hot reload or is it automatic? @4ian
I'm asking here, but I'm interested for my bitmapText.

@HarsimranVirk
Copy link
Contributor Author

I'm thinking about it now but do you also support hot reload or is it automatic?

@Bouh thanks for reminding this, I had to implement it as well 💀.

I'm asking here, but I'm interested for my bitmapText.

I guess all we need to do is define a function updateFromObjectData, and return true if the properties are updated correctly. Otherwise, return false, in which case the user would have to restart the preview. Take a look at other objects for implementing this function (maybe BBText since that would correlate a bit to bitmapText? 😬).

@4ian
Copy link
Owner

4ian commented Aug 7, 2020

Correct, implement updateFromObjectData to get hot-reload support. Check how it's done for other objects (compare the properties, change anyone that must be updated, then return true. return false if something is not supported, in which case the user will be asked to relaunch the preview. This is the case by default if you don't implement updateFromObjectData).

EDIT: Also in the future maybe a function like updateFromObjectData could be used for things like updating an object from the network, etc..

@4ian
Copy link
Owner

4ian commented Aug 12, 2020

I'm doing a new review now and we should prepare for merging this, I'll give this another try too :)

@4ian 4ian added the 🏁PR almost ready: final fixes The PR is almost ready and needs final fixes and/or polishing before merge label Aug 12, 2020
@4ian
Copy link
Owner

4ian commented Aug 12, 2020

A few more comments after testing:

  • Changing the ambient light and hot reloading => not applied. You might need to do something in gdjs.HotReloader.prototype._hotReloadRuntimeLayer.
  • Running a preview with just a light. Then add a Light Obstacle Manager to an object, and hot-reload => the behavior is not applied.
    • This is probably because the object can't find the light obstacle manager. Solution: always include all the files, including the lightobstacleruntimebehavior.js, even in the includes of the object.
  • You need to implement onActivate and onDeActivate for your behavior. See how it's done for gdjs.PlatformRuntimeBehavior, otherwise activating/disabling the behavior using events won't stop blocking the light.
    • EDIT: and onDestroy too!

Let's solve all of this and the other comments in the code and we should be good to go with this :)

@4ian
Copy link
Owner

4ian commented Aug 12, 2020

Speaking of onDestroy, I made a fix so that removing a behavior and launching a hot-reloading is properly calling onDestroy (so that for example, platforms don't stay as platforms after removing the Platform behavior) in 4324526. I surely want to merge master again :)

@HarsimranVirk
Copy link
Contributor Author

HarsimranVirk commented Aug 12, 2020

@4ian quick question:

I made a fix so that removing a behavior and launching a hot-reloading is properly calling onDestroy

Does it require rebuilding libGD?

EDIT: EventsCodeGenerator.cpp was modified recently, it's maybe related to the removal of Array prototype methods 🤔. Either way, it needs to be rebuilt then 😅.

@4ian
Copy link
Owner

4ian commented Aug 12, 2020

Yep there was a change in the code generation!
As for lighting, nothing in libGD, this is all in the extension "runtime" code. Adding these methods us enough for the game engine to call them.

@4ian 4ian merged commit b7902bb into 4ian:master Aug 17, 2020
@4ian
Copy link
Owner

4ian commented Aug 17, 2020

Finally merged! :)
This seems to work well enough for us to go ahead with this! Thanks a lot for all the work made on this so far. It's a great feature that will certainly get a lot of love from users and will unlock very nice effects and new gameplay possibilities :)

Let's see what we can achieve more in the remaining days of GSoC!

@HarsimranVirk HarsimranVirk deleted the lights-master branch August 18, 2020 07:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏁PR almost ready: final fixes The PR is almost ready and needs final fixes and/or polishing before merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

karma test command doesn't work in windows
5 participants