Skip to content
This repository has been archived by the owner on Apr 22, 2021. It is now read-only.

Xray #447

Merged
merged 3 commits into from
Feb 17, 2020
Merged

Xray #447

merged 3 commits into from
Feb 17, 2020

Conversation

5HT2
Copy link
Member

@5HT2 5HT2 commented Feb 17, 2020

Closes #289
Unblocks #11, #394 and #429

The API isn't used yet, but it's an obvious "add commands to run these" marker
@5HT2 5HT2 added module/new-feature --review-pending Pending a review of the pull labels Feb 17, 2020
@5HT2 5HT2 added this to the v1.1.3 milestone Feb 17, 2020
@5HT2 5HT2 self-assigned this Feb 17, 2020
Comment on lines +25 to +28
if (!xr.isEnabled()) {
Command.sendChatMessage("&cWarning: The XRay module is not enabled.");
Command.sendChatMessage("&cThese commands will still have effects, but will not visibly do anything.");
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Reminder for myself to change this to send warning message

Comment on lines +21 to +24
if (xr == null) {
Command.sendChatMessage("&cThe XRay module is not available for some reason.");
return;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This to send error message

Comment on lines +14 to +16
super("xray", new ChunkBuilder().append("subcommand").build());
setDescription("Has a set of sub-commands to control the XRay module.");
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Better description and aliases

@5HT2
Copy link
Member Author

5HT2 commented Feb 17, 2020

Also add a xray thing to .help if it doesn't already have a proper help I'm sleepy

@5HT2
Copy link
Member Author

5HT2 commented Feb 17, 2020

Also, if a minecraft: isn't specified autocomplete it when saving and removing from the list

@5HT2 5HT2 added 3 - Review and removed new/gui labels Feb 17, 2020
This was referenced Feb 17, 2020
@20kdc
Copy link
Contributor

20kdc commented Feb 17, 2020

The minecraft: prefix is optional due to the way the get-block-by-name function works; it's sanitized on addition or removal of elements from the list using the .xray +block -block syntax.

@5HT2
Copy link
Member Author

5HT2 commented Feb 17, 2020

Oh it already is?

@20kdc
Copy link
Contributor

20kdc commented Feb 17, 2020

Yes, but since it's an internal detail of the setting the relevant code is in the XRay module itself.

@5HT2
Copy link
Member Author

5HT2 commented Feb 17, 2020

Oooh ok, thank you, that's good to know.

@5HT2
Copy link
Member Author

5HT2 commented Feb 17, 2020

Now this is only waiting for 1 merge conflict and to change the stuff to sendErrorMessage and everything. I'll go eat something first I guess

@20kdc
Copy link
Contributor

20kdc commented Feb 17, 2020

To be specific:
https://github.com/S-B99/kamiblue/pull/447/files/9c774ffc93cbea58e5e363401ea4c2dd5f090c42#diff-7c8abe7a172750487fdbbfb9dffc8557R73

  1. If a value is added to the list, it may end up without the minecraft: prefix, but it gets sanitized on the next pass, and the relevant functions handle it there.
  2. If a value is removed from the list, it's removed by-Block-instance so how it's expressed or any aliases don't matter.
  3. When the list is being read, it's (again) converted to Block instances.

@20kdc
Copy link
Contributor

20kdc commented Feb 17, 2020

A note: The name of the block states/etc. is dependent on mod ID, which might cause trouble for the merge.

@5HT2
Copy link
Member Author

5HT2 commented Feb 17, 2020

Huh, where is that defined? And why is it dependant on it?
Unless you modified the modid lines it should be fine?

@20kdc
Copy link
Contributor

20kdc commented Feb 17, 2020

The XRay defines a block. Check resources. The Java code will be fine, but the block JSON has kami: references all over it...

@5HT2 5HT2 merged commit 9c774ff into kami-blue:feature/master Feb 17, 2020
@20kdc
Copy link
Contributor

20kdc commented Feb 17, 2020

Er, just to be clear, the current state of feature/master will probably malfunction as the kami references have not been changed to kamiblue.
These are the resources/assets/kami directory and the JSON files in there.

@5HT2
Copy link
Member Author

5HT2 commented Feb 17, 2020

If you see in #289 I just changed the references, but I should be using kamiblue for the folder name as well. Just curious, why did you do those json things instead of using a ResourceLocation?

@20kdc
Copy link
Contributor

20kdc commented Feb 17, 2020

If ResourceLocations are able to dynamically define blocks without any JSON support files, then it'd be good to switch to that; I wasn't aware of it and went with the "default" method for adding a new block.

@5HT2
Copy link
Member Author

5HT2 commented Feb 17, 2020

Ah. I'm not sure if how exactly this works, I haven't looked to closely at it but basically you can just open a resource location to a file and use it as a texture, look at how InventoryPreview does it.

@20kdc
Copy link
Contributor

20kdc commented Feb 17, 2020

Using it as a texture, yes, but the XRay doesn't actually do any rendering, as that could cause problems with OptiFine/etc.
Instead the XRay changes which block states the renderer gets. This implies anything that the XRay sends to the renderer has to be in the form of a valid block state.

@5HT2
Copy link
Member Author

5HT2 commented Feb 17, 2020

oooh ok. makes sense now c:

@nehemiasjoelwww

This comment has been minimized.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Xray
3 participants