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

Transparent Boxes With proper merge #23

Closed
wants to merge 3 commits into from
Closed

Transparent Boxes With proper merge #23

wants to merge 3 commits into from

Conversation

XeroOl
Copy link
Contributor

@XeroOl XeroOl commented Aug 31, 2015

I believe this should work properly. The boxes are coded correctly. The connector code has been changed so it doesn't render connectors behind the box. I added a color scheme of green to yellow.

XeroOl added 3 commits August 29, 2015 16:39
Renders Exits/Portals through blocks.
Renders Snakes through blocks. (the no-collision cheat)
Renders other block's connectors through blocks.
Doesn't render its own connector inside the block.
I like these colors the best
It looks like you re-formatted my code (replacing 1%2 with 1 % 2,
replacing tabs with spaces, etc)Probably was like an automatic formatter
for whatever editor you use. It thought that we were both modifying the
same lines beacause of this. This update should fix the conflicts
@XeroOl XeroOl mentioned this pull request Aug 31, 2015
@XeroOl
Copy link
Contributor Author

XeroOl commented Aug 31, 2015

Also, it looks like this includes your issue #9 work too, so you might want to switch that work back to master.

@thejoshwolfe
Copy link
Owner

The issue #9 work is incomplete. I doesn't belong in master yet. It literally has a button that throws an exception.

I think you should take out the issue #9 work from this pull request so that it can be merged into my master.

Also, you don't have to make pull requests from your master branch. I usually like to make a branch for each pull request I make (the branch is named "pull1" or something.). That way I can keep working on master in my fork without changing what the pull request refers to. You can also use this strategy to make multiple pull requests at once, although it's a bit tricky to keep them independent of each other.

@thejoshwolfe
Copy link
Owner

I don't think you need to make a new pull request each time you change what commits are included in it. I think the pull request will follow whatever is in your branch that you designated as the branch for this pull request, which is your master branch. If you just change your master branch, it should update what the pull request entails.

@XeroOl
Copy link
Contributor Author

XeroOl commented Sep 1, 2015

I had the transparent stuff done when you pulled the colored box one, and those were both on master... I don't think it auto-updates.

edit: nevermind.

@XeroOl
Copy link
Contributor Author

XeroOl commented Sep 1, 2015

Ok I undid the merge manually. this should be OK now

@thejoshwolfe
Copy link
Owner

You don't have to undo the changes manually. Looking at the history, it looks like commit 0e8af7f8b4f3ef7a11197b5bbab6ff44553eb22c was right before merging my issue-9 branch. If you do this command:

git reset 0e8af7f8b4f3ef7a11197b5bbab6ff44553eb22c --hard

then it will forget that the merge ever happened, erasing it from your local history. Then in order to tell github about going back in time, you need to do a special kind of push:

git push --force

(I seem to remember github having a setting to completely disallow this kind of push, so you might need to enable that as well. I'm not sure.) You have to use --force to tell github that you know how serious going back in time is. It's serious because it's hard to undo and will often cause collaborators to have problems. But since we're the only two collaborators and we're on the same page, that shouldn't be a problem. And since the whole point of this back-in-time push is to erase a series of commits that cancel each other out, there shouldn't be any worry about losing that work forever.

I suggest doing all this to clean up the history. I think, as it is now, your changes will cause conflicts when I try to merge my issue-9 branch in for reals. Telling git to forget about the whole thing will look cleaner and behave better in the long run.

@XeroOl
Copy link
Contributor Author

XeroOl commented Sep 1, 2015

ok right now I don't have access to the git client, but I will do that when i get home. (about 3 hours from now)

@XeroOl
Copy link
Contributor Author

XeroOl commented Sep 1, 2015

ugghh you can't copy+paste into git's shell

@XeroOl
Copy link
Contributor Author

XeroOl commented Sep 1, 2015

Ok its gone

@XeroOl
Copy link
Contributor Author

XeroOl commented Sep 1, 2015

Again, All the features in this request.

The transparent box version's features:
7 Block colors!
image
Tiles render through block!
image
Snakes can be seen inside block! (reproduce with the collision turned off)
image
Connectors render properly!
image
Connectors render through other blocks!
image
The block in the cursor is the color of the block you will place!
image
Sky inside the selected box is also darkened!
image
I will make the connectors slightly darker than the box right now.

@thejoshwolfe
Copy link
Owner

I will make the connectors slightly darker than the box right now.

Are you waiting for me to merge this pull request first?

@XeroOl
Copy link
Contributor Author

XeroOl commented Sep 3, 2015

I thought I already pushed that? Can't seem to find the color update. I guess I haven't done that yet... The colors should be pretty easy to change later if you want to merge it now.

@XeroOl
Copy link
Contributor Author

XeroOl commented Sep 3, 2015

Also, what software do you use to code? I am using notepad++.

@thejoshwolfe
Copy link
Owner

Also, what software do you use to code? I am using notepad++.

Vim in Linux, although I can't say I necessarily recommend it. I recently got my friend set up using Eclipse for JavaScript development, ... and I don't really recommend that either.

Actually, Notepad++ is pretty solid. There are a few configuration defaults I don't like, like I always turn hard tabs off, and indentation width to 2 instead of 4. That's my preference, but it's also standard in this codebase, so it might help relieve some frustration if you find yourself fighting with the indention every time you hit Enter or Tab.

I've heard Jonathan Blow is planning to make a text editor sometime in the next couple years. I'm pretty excited to see how that goes. His plans are a big reason why I'm not trying to make a text editor of my own anytime soon. I pretty much hate all existing text editors and IDEs :/

@thejoshwolfe
Copy link
Owner

I thought I already pushed that? Can't seem to find the color update.

Oh, oops. I may have instructed you to erase it from history. If you did it before the git reset ... command, you probably lost it there.

Also, I meant to mention that you can paste into git's shell (really the Windows terminal emulator) with Alt+Space, E, P (took me years to find that out.). You can't use the standard clipboard hotkeys (Ctrl+C, Ctrl+V, etc.) in any terminal emulator I'm aware of, including Console2, Putty, Mintty, or anything on Linux either. Sometimes there are alternative hotkeys, like Shift+Insert or Ctrl+Shift+V, but there's almost always a menubar way to do it as well that should tell you the hotkeys.

@Patashu
Copy link

Patashu commented Aug 11, 2016

Is this good to merge, thejoshwolfe? Transparent blocks are necessary to be able to see the tile behind a block, which is important once we get multiple types of non-air terrain that you can stand in.

@thejoshwolfe
Copy link
Owner

unfortunately, this pull request has suffered from bitrot, and is not easily mergeable into the current codebase. I've reimplemented this feature on top of the current codebase using a slightly different approach: using the .clip() api to stencil out any support beams that would be visible behind the transparent blocks.

screenshot_2016-08-05_11-34-59

We can still do more with the aesthetics, like making the outlines thicker, changing the colors, or adding more colors.

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