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

Improve existing D6s and add a traditional D6 #55

Merged
merged 2 commits into from
Apr 17, 2021
Merged

Improve existing D6s and add a traditional D6 #55

merged 2 commits into from
Apr 17, 2021

Conversation

20kdc
Copy link
Contributor

@20kdc 20kdc commented Apr 15, 2021

Adds a "traditional" D6 (standard dice), and adds borders around the others to work around the lack of a way to turn off texture filtering.

image

Fixes/Solves
Might partially solve D6 requirements for #28

@drwhut
Copy link
Owner

drwhut commented Apr 15, 2021

Thank you for the PR!

I've been meaning to replace these textures for a while now (these were the very first textures I used to test custom assets, I believe), but have just never gotten around to it.

I agree there should be a traditional D6 dice, but I personally think it would look better if the black squares were circles instead.

Now that I think about it, if you're willing to do something extra, maybe instead of having multi-coloured die, each die should have one colour (e.g. red, blue, white etc.), with numbers on them?

@20kdc
Copy link
Contributor Author

20kdc commented Apr 15, 2021

While I do want separate coloured die for my own reasons, I think I'd need better access to Godot material properties to do so flexibly.
The problem is the current asset pack design does not facilitate convenient in-Godot editing of Godot files.

@drwhut
Copy link
Owner

drwhut commented Apr 15, 2021

Good point! I was just thinking about making separate textures for each of the colours, but now that you've mentioned that I think it would be more convenient if the albedo colour could be changed in-game...

@20kdc
Copy link
Contributor Author

20kdc commented Apr 15, 2021

I wasn't thinking the albedo colour as a whole. I was thinking more like allowing general use of more of the materials framework rather than just pure albedo - a "mask albedo" similar to some and an additional roughness map would technically work, but general flexibility would be nice.

@drwhut
Copy link
Owner

drwhut commented Apr 15, 2021

Gotchya. I agree that more of the material properties should be exposed (e.g. the normal map, the roughness etc.), but I think fully exposing all material properties at once would cause more problems than it solves - I think it makes more sense to expose certain properties if the need for them is high enough. Speaking of which, which properties do you think should be exposed?

@20kdc
Copy link
Contributor Author

20kdc commented Apr 15, 2021

Then at least everything required for PBR, plus additive masks for colouration on albedo & emissive...
If this sounds overtly complex, then hopefully you get why I think using Godot's material system makes more sense (isn't there already "use scene" functionality, just a pain to use?)

@drwhut
Copy link
Owner

drwhut commented Apr 15, 2021

Technically, you already can modify all of these properties if you import a custom 3D model with its own materials. However, as of right now you cannot import 3D models as dice (although now that I think about it you should be able to, all that's special about dice is that they change rotation when shaken).

Hmm... now I'm thinking, if someone knows they want to change a specific texture property, then chances are they already know a bit about 3D modelling, and can export a 3D model. Otherwise, they can stick with the regular method and make an image (which is used by the game as an albedo texture). So maybe it's pointless to expose these properties if they are already (indirectly) exposed via 3D models?

Also, I just noticed you modified the traditional D6, thank you!

@20kdc
Copy link
Contributor Author

20kdc commented Apr 15, 2021

On another note, I have two d20s here...

one's unfinished and doesn't fit with other assets:
image

the other's pretty much just like the other dice and is implemented the same way:
image

Might as well ask: Which to PR, both, etc, separately?

@drwhut
Copy link
Owner

drwhut commented Apr 15, 2021

Thanks! I'd say upload both of them in a ZIP file to this thread - I need to add the meshes to the game (since there are no d20 currently), and I'll also need to add the UV mapping to the docs.

@20kdc
Copy link
Contributor Author

20kdc commented Apr 15, 2021

I already have the AssetDB/etc. integration for the second ready.

@20kdc
Copy link
Contributor Author

20kdc commented Apr 15, 2021

This contains a GLB for the first and a patch for the second. Note that the first model remains incomplete due to the lettering being applied non-ideally.
d20s.zip

@drwhut
Copy link
Owner

drwhut commented Apr 15, 2021

Wow! Looking at the patch, you got the integration pretty much perfect! The only thing, and this is a very minor thing, is the UV mapping is very awkward (makes sense, given it's what "Blender knew best") which would make creating d20s without 3D modelling software difficult, but it can always be adjusted afterwards, so it's not that big of a deal.

@20kdc
Copy link
Contributor Author

20kdc commented Apr 16, 2021

Actually, the UV mapping is my own. The one Blender made was arguably worse.
The idea behind the UV mapping I chose was to keep everything in an upwards orientation even if the results were more scrambled.

@drwhut
Copy link
Owner

drwhut commented Apr 16, 2021

Ah, I thought it was Blender's based on the editor comment in the patch diff.

That reasoning makes sense, my only concern is that compared to the current UV mappings, the triangles are very skewed. Ideally, it would be just as easy to make a d20 as it would be to make a d6 or d8 with just a photo editor.

d20:
d20_mapping

d8:
d8

But like I said, the UV mapping can be changed at any time, so it's nothing to worry about!
By the way, feel free to make another PR for the blank d20 if you want, and we can continue this thread there instead.

@20kdc
Copy link
Contributor Author

20kdc commented Apr 16, 2021

oh, wait, you mean the UV mappings on the fancy D20?
Those mappings are done in "lightmap pack" mode w/ Blender. They were not intended for end-user modification. Try the other one.

@drwhut
Copy link
Owner

drwhut commented Apr 16, 2021

Ah, sorry for the confusion! Yeah, I'm happy with this UV mapping:
d20_mapping_new

The only thing I can think of is maybe the triangles could be bunched up together so there's less empty space in the texture (similar to the d8 mapping)? What do you think?

@20kdc
Copy link
Contributor Author

20kdc commented Apr 16, 2021

It's a good idea in theory, but in practice it means editing the UVs without making the margin situation worse than it already is (keep in mind the margin problems the D6 UV mapping has), and editing UVs manually while preserving sizes is a tad annoying. In practice this "grid layout" allows copy/pasting the same square texture piece, which is useful.

@drwhut
Copy link
Owner

drwhut commented Apr 16, 2021

Good point - whichever direction the UV mapping goes for the d20 (less empty space vs. better margins), the same will need to be done for all of the other UV mappings. If you want, I'd be happy to merge in the d6's and d20 (the blank one) as they are, then I'll modify the UVs afterwards so they are all consistent?

@20kdc
Copy link
Contributor Author

20kdc commented Apr 16, 2021

Keep in mind that there is no need for the UVs in the "Fancy D20" (non-blank) to be consistent with other UVs. The "Fancy D20" UV map is solely designed for being baked from another source.
Anyway, that's fine, yeah

@drwhut drwhut merged commit f2f250d into drwhut:master Apr 17, 2021
@drwhut
Copy link
Owner

drwhut commented Apr 17, 2021

Thank you! 😄

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

Successfully merging this pull request may close these issues.

2 participants