Iron chunks now use visual identificator #5409
Conversation
|
We are currently in feature freeze until the next release. |
There was a problem hiding this comment.
Here's some comments. And also when reading this I realized that the CollisionShapeLoader component has a hardcoded path to a physics data object. Meaning that just these changes aren't sufficient to protect the chunks from save breakage when moving resource files around.
| @@ -46,19 +46,19 @@ | |||
| "Name": "SMALL_IRON_CHUNK", | |||
| "Meshes": [ | |||
| { | |||
| "ScenePath": "res://assets/models/IronRock1.tscn", | |||
| "VisualResource": 9, | |||
There was a problem hiding this comment.
Yeah this, is not very nice because people need to manually write this file so using string keys here would be vastly superior.
When referring to the integer values I meant how this will be saved in the game saves, which are not edited by humans so the compactness of just pure numbers helps there.
There was a problem hiding this comment.
So what do I put here instead if visual id is BigIronChunk1?
There was a problem hiding this comment.
"BigIronChunk1"
which either works immediately or a custom JSON converter is needed that supports both string and integer values.
There was a problem hiding this comment.
JSON converter is needed then. Unfortunately.
There was a problem hiding this comment.
I just looked and so far it seems like no enum values have been wanted to read from strings in the game. The RevolutionaryWebApp repo has that setup already, but sadly it uses a different JSON lib, so a separate approach is needed here.
| @@ -146,6 +146,8 @@ public class ChunkScene | |||
| /// </summary> | |||
| public string ScenePath; | |||
There was a problem hiding this comment.
This and the loaded scene have to be removed in order to not cause confusion by having 2 separate mechanisms at once specifying how the visual is defined.
There was a problem hiding this comment.
This will cause to at least chunks on microbe death not function. Last time I tried to do it with fixing said issue, it froze my game.
There was a problem hiding this comment.
Those chunks must also be updated to the new system. I won't have the confusion of having 2 systems for cell chunk visuals.
| { | ||
| ScenePath = selectedMesh.ScenePath, | ||
| VisualIdentifier = meshes[random.Next(meshes.Count)].VisualResource, |
There was a problem hiding this comment.
This should use the selected mesh because otherwise the visual mesh may not match what was selected as the physics mesh.
Brief Description of What This PR Does
This PR does some stuff...
Related Issues
Closes #5402
Progress Checklist
Note: before starting this checklist the PR should be marked as non-draft.
break existing features:
https://wiki.revolutionarygamesstudio.com/wiki/Testing_Checklist
(this is important as to not waste the time of Thrive team
members reviewing this PR)
styleguide.
Before merging all CI jobs should finish on this PR without errors, if
there are automatically detected style issues they should be fixed by
the PR author. Merging must follow our
styleguide.