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

[4.0] Clean up and fix Truck Town #767

Merged
merged 1 commit into from
Aug 30, 2022

Conversation

jtnicholl
Copy link
Contributor

I've cleaned up the Truck Town demo and gotten it fully working.
I moved all of the mesh data out of the .tscn files into binary resource files. Except for the town itself, which I brought into Blender and did a clean up on; I fixed the broken normals that were messing up the lighting and also de-duplicated the meshes. It's now a .glb file.
I also did a bit of code cleanup, made the file paths lowercase, updated the screenshot and icon, and turned on FXAA.

Copy link
Member

@aaronfranke aaronfranke left a comment

Choose a reason for hiding this comment

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

This looks really good from a quick review, thank you!

I noticed that the choose_* files got bigger. You can use a tool called oxipng to compress PNG images, the command is oxipng -o6 --strip all --zopfli --fix.

3d/truck_town/vehicle.gd Show resolved Hide resolved
@jtnicholl
Copy link
Contributor Author

I noticed that the choose_* files got bigger. You can use a tool called oxipng to compress PNG images, the command is oxipng -o6 --strip all --zopfli --fix.

That's what I did, in order to optimize the new icon and screenshot. I ran it over the rest of the files again and there was no difference.
Viewing the commit in GitHub it seems to be saying the files went up in size by their total size, I think that's a bug in GitHub. The only change I made to those files was move them.

@jtnicholl jtnicholl force-pushed the truck_town branch 2 times, most recently from 88a5313 to 726d4f4 Compare August 26, 2022 06:13
Copy link
Member

@aaronfranke aaronfranke left a comment

Choose a reason for hiding this comment

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

Did a quick test. There are some problems. The trucks do not spawn in the correct place, and the joints are missing. These problems do not exist on the current 4.0-dev branch which has Truck Town ported already.

@jtnicholl
Copy link
Contributor Author

Are you sure you're on the right branch? The joints are working fine when I test it. But when I test the current 4.0-dev branch the joints aren't attached. Also, the trucks are spawning in the same place on both branches. Where are they supposed to spawn?

@jtnicholl
Copy link
Contributor Author

jtnicholl commented Aug 26, 2022

Ok, I just tested with alpha 14 instead of building from latest master, and now I'm seeing the issues you mentioned. It looks like the node params on Joint got renamed some time between alpha 14 and now. As for spawning in the wrong location, I don't know why that happens, but it's fixed if you build from latest master.
Edit: The trucks spawning in the wrong place is because InstancePos was a Position3D, and that got renamed to Marker3D. Opening in alpha14 it shows up as a MissingNode, which does not extend Node3D, so it puts the truck at (0, 0, 0).

@aaronfranke
Copy link
Member

You're right, it does work with master. However, with master I noticed that there are weird shadows everywhere? It happens on the 4.0-dev branch and with your PR with Godot master, but it doesn't happen with alpha 14. Anyway, this isn't a problem with your PR, so we can merge it after alpha 15 is released.

Screen Shot 2022-08-26 at 1 21 38 PM

@Calinou
Copy link
Member

Calinou commented Aug 26, 2022

It happens on the 4.0-dev branch and with your PR with Godot master, but it doesn't happen with alpha 14.

This is a engine bug from the recent merge of octahedral compression: godotengine/godot#64854

@jtnicholl jtnicholl force-pushed the truck_town branch 2 times, most recently from 1ce80f9 to 8c299ee Compare August 30, 2022 00:53
@jtnicholl
Copy link
Contributor Author

jtnicholl commented Aug 30, 2022

I just cleaned up the vehicles and put them in a glb file as well, to fix the octahedral compression errors, a hole on the tow truck, and some backwards normals on the trailer. Also improved the wheels and branded the sides of the trailer with a waiting for Godot joke while I was at it.

@aaronfranke aaronfranke merged commit cd586ac into godotengine:4.0-dev Aug 30, 2022
@aaronfranke aaronfranke added this to the 4.0 milestone Aug 30, 2022
@aaronfranke
Copy link
Member

Thanks! I made a follow-up PR: #768.

@jtnicholl jtnicholl deleted the truck_town branch September 9, 2022 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants