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

New images #56

Merged
merged 4 commits into from
Nov 19, 2013
Merged

New images #56

merged 4 commits into from
Nov 19, 2013

Conversation

Lowest0ne
Copy link
Member

Fixes #40

I know they aren't pretty ( but they sure won't fall over ). That is about the best of my ability. It would probably be wise to find a set online that we know we can use without worry.

I also removed the chessboard that was at the wrong rotation.

Making 12 images that are lined up is a PIA so I made a support program to split one png into the 12 that we need. The program requires png++ and libpng, I imagine anyone who wants to use it can manage finding them on his/her own.

@LB--
Copy link
Member

LB-- commented Nov 18, 2013

I don't think new_pieces.png and png_split.cpp should be included. png_split.cpp especially is not related to this project, and would do well as its own repository. Even if you were to integrate ng splitting into the graphics module, I'd be against this because it makes adding new pieces difficult and the images would have to be sent one by one over the network either way when networking is implemented.

The pieces are nice, but I don't like the knight design - is it supposed to be a blocky saddle?

@Lowest0ne
Copy link
Member Author

Lol, no, that's a horse's head :)

I put the image and the source in the commit because it makes it easier for someone to edit them. I certainly wasn't going to try to match 12 bases that were in separate files.

Can I make a repo here ( cpluspluscom )? Might be neat to have a developer tools repo.

@LB--
Copy link
Member

LB-- commented Nov 18, 2013

Yes, feel free to make a repository here! That's what this organization is for :)

@ghost
Copy link

ghost commented Nov 18, 2013

First would like to say I like the new pieces and I think they would definitely work for the time being. Also I am curious why not use sprite sheets like new_pieces.png instead of single piece textures? Much better to load 1 texture for the pieces then 12.

Also the developer tools repo sounds like fun. What exactly do you have planned for it? Like just a collection of useful classes, functions, ect that people can add to and use? Or something else?

@LB--
Copy link
Member

LB-- commented Nov 18, 2013

Also I am curious why not use sprite sheets like new_pieces.png instead of single piece textures? Much better to load 1 texture for the pieces then 12.

Then we support only an exact set of pieces with no ability to support additional pieces without modifying the image. One of the original goals of the project is modularity and supporting many of the other chess variants.

@LB--
Copy link
Member

LB-- commented Nov 18, 2013

I created the repo for you using the subtree split method:
cpluspluscom/dev-tools@be6a1e7

@Lowest0ne
Copy link
Member Author

I made the commit to remove .cpp and image last night, so it appears earlier in this conversation.

Thanks for the repo :)

@LB--
Copy link
Member

LB-- commented Nov 19, 2013

I think this commit is good to merge despite the knight being a minecraft saddle, just want to get @Thumperrr's and @naraku9333's opinions.

@naraku9333
Copy link
Member

They look good to me.

@Thumperrr
Copy link
Member

I'm also on board with this.

@Thumperrr Thumperrr closed this Nov 19, 2013
@Thumperrr Thumperrr reopened this Nov 19, 2013
Thumperrr pushed a commit that referenced this pull request Nov 19, 2013
@Thumperrr Thumperrr merged commit f91e31d into cpluspluscom:master Nov 19, 2013
@LB-- LB-- mentioned this pull request Nov 19, 2013
@Lowest0ne Lowest0ne deleted the new-images branch November 19, 2013 23:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

License
5 participants