fix: make ChangingBlocks case insensitive via BlockUri #15
Conversation
- before this PR, a prefab specifying the `ChangingBlocks` component had to make sure that the keys in the `blockFamilyStages` map match the string representation of the respective `BlockUri`s including case example for a mismatch: `PlantPack:Corn1` (as referenced in `PlantPack:Corn.prefab`) != `PlantPack:corn1` (in-game `BlockUri` of `PlantPack:Corn1.block`) - by sanitizing the reference in the `ChangingBlocks` component via the `SimpleUriTypeHandler` and comparing it with the `SimpleUri` derived from the in-game `BlockUri`, the logic becomes case insensitive
jdrueckert
added
Type: Bug
- line length - missing whitespaces - copyright updates
| @@ -63,7 +51,7 @@ public void onSpawn(OnAddedComponent event, EntityRef entity) { | |||
| ChangingBlocksComponent changingBlocks = entity.getComponent(ChangingBlocksComponent.class); | |||
| LocationComponent locComponent = entity.getComponent(LocationComponent.class); | |||
| Block currentBlock = worldprovider.getBlock(locComponent.getWorldPosition(new Vector3f())); | |||
| String currentBlockFamilyStage = currentBlock.getURI().toString(); | |||
| SimpleUri currentBlockFamilyStage = new SimpleUri(currentBlock.getURI().getModuleName(), currentBlock.getURI().getIdentifier()); | |||
There was a problem hiding this comment.
Maybe this should be available on Block or BlockUri to prevent accidental errors. 🤔 (I'd use at least a module-wide helper class to be sure that we're using the same conversion within the context of Changing Blocks). However, I'm not sure how universal (and generally helpful) this reduction of information is. So, probably the module-wide helper function would indeed be a good compromise.
(We talked about this offline, but just for the record)
The culprit here is that the block uri can contain way more information (shape, orientation, ...) than just the plain block "type" (family?).
Even more confusing, it seems like we're hiding some shapes (if the shape is hard-coded in the block definition maybe?)
|
Wait, there's a Okay, yes, I see now that there is. Did I mislead you then, and you actually want be using that instead of SimpleUri? Looking at the implementation of BlockUri, I see there's also gestalt.assets.ResourceUrn. |
| Block newBlock = blockManager.getBlock(newBlockUri); | ||
| if (newBlockUri.equals(newBlock.getURI().toString())) { |
There was a problem hiding this comment.
BlockManager#getBlock cannot return null. If, before returning, the block that would be returned equals null the air block is returned instead.
jdrueckert
changed the title
Fixes #14
Depends on MovingBlocks/Terasology#5061
Contains
ChangingBlockscomponent had to make sure that the keys in theblockFamilyStagesmap match the string representation of the respectiveBlockUris including caseexample for a mismatch:
PlantPack:Corn1(as referenced inPlantPack:Corn.prefab) !=PlantPack:corn1(in-gameBlockUriofPlantPack:Corn1.block)ChangingBlockscomponent via theBlockUriTypeHandlerand comparing it with the in-gameBlockUri, the logic becomes case insensitiveHow to test
Requires MovingBlocks/Terasology#5062
CoreGameplayand add thePlantPackandChangingBlocksmodules in the Advanced Game Setup (AGS)give corn1Outstanding before merge
ChangingBlocksusages other thanPlantPackand create follow-up issues to adjust map formattingChangingBlockscomponent