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

Implemented simple //write for binary values (#268) #269

Open
wants to merge 15 commits into
base: dev
Choose a base branch
from

Conversation

DreiMu
Copy link

@DreiMu DreiMu commented Jun 7, 2023

Simple implementation for a write command, still needs testing, only bug found for now is the redstonelamps turning off, if there is one updated next to them

@Matthias1590
Copy link
Member

The default on block should be a redstone block, the fact that lamps turn off when updated is normal

@Matthias1590
Copy link
Member

Looks good, lmk when you're done editing & testing

@DreiMu
Copy link
Author

DreiMu commented Jun 8, 2023

I can't find any more errors and have nothing more to implement, somebody else should still test it though

@DreiMu DreiMu marked this pull request as ready for review June 8, 2023 21:04
@Matthias1590
Copy link
Member

Matthias1590 commented Jun 9, 2023

image
Not entirely sure whats going on here, it seems to be writing more bits than would fit, if I make the offset 1 it works, maybe it should have a bitsize instead of offset and then just calculate the offset using the bitsize and selection size

@Matthias1590 Matthias1590 added pr/needs-testing The funcitonality added needs to be tested and removed pr/needs-testing The funcitonality added needs to be tested labels Jun 9, 2023
Copy link
Member

@Matthias1590 Matthias1590 left a comment

Choose a reason for hiding this comment

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

Still has some issues, see comment above

…re/binary-write

# Conflicts:
#	src/main/java/tools/redstone/redstonetools/features/commands/BinaryBlockWriteFeature.java
@DreiMu
Copy link
Author

DreiMu commented Jun 10, 2023

They should be fixed now

@DreiMu DreiMu requested a review from Matthias1590 June 10, 2023 13:40
@Bitlett
Copy link
Contributor

Bitlett commented Jun 11, 2023

Please mention the issue next time so your PR shows up on the issue.

#268

@DreiMu
Copy link
Author

DreiMu commented Jun 11, 2023

I had the issue in the name, so that doesn't work, so I'll do it next time in the comment

@Matthias1590
Copy link
Member

image
This should probably return Feedback.invalidUsage stating that the number is too big for the bit count.

Also
image
I think we should just put a minimum on the number so you cant write negatives for now

@DreiMu
Copy link
Author

DreiMu commented Jun 12, 2023

fixed both problems

@DreiMu DreiMu requested a review from Matthias1590 June 12, 2023 22:53
@Matthias1590
Copy link
Member

This behaves rather weirdly imo, when you select 7 blocks (b a b a b a b, where b is a block and a is air) and try to write a 4 bit number it'll tell you the selection isnt a multiple of the bit count when I think its a perfectly valid input. I think we're gonna have to postpone this feature and add it after the refactor

@Matthias1590 Matthias1590 added this to the Post refactor milestone Jun 19, 2023
@DreiMu
Copy link
Author

DreiMu commented Jun 19, 2023

It was pretty easy to fix, if you want to test it again, it should now work

@Matthias1590
Copy link
Member

Not sure whether this is an issue with this feature or not but could you look into this?
image

@DreiMu
Copy link
Author

DreiMu commented Jun 19, 2023

This seems to be a problem of the feedback formatting, the only thing I could do about it, is to shorten the text

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.

3 participants