antirez / load81

SDL based Lua programming environment for kids similar to Codea
BSD 2-Clause "Simplified" License
647 stars 64 forks source link

Initial alpha value should be 255 (not 1) after change from float to int #30

Closed ghost closed 12 years ago

antirez commented 12 years ago

Hey, good catch! Please try to create a branch that is obtained by forking the current master on github, and re-apply the patch and then send your pull request again so that it includes only the fixing commit. Thank you! (Otherwise I can merge it anyway but it's not the proper way of doing it as you'll have again the same problem of not being albe to pull).

ghost commented 12 years ago

I thought this is what I was doing. I forked the project originally and I don't to anything on the master branch. I keep doing:

git merge upstream/master

then I created a branch off that, applied by patch, committed and then made a pull request. What should I be doing?

antirez commented 12 years ago

You should do the git reset --hard thing using the latest SHA on github, and then create a branch with "git branch", and finally create the patch, push, and make the pull request, and you'll see just one commit in the list of commits of the pull request.

Otherwise if it gets complex just remove the old source tree and clone it from scratch :)

ghost commented 12 years ago

closing pull request. trying again.

seclorum commented 12 years ago

Just FYI, I've been cloning the master branch every time I make a change to submit a pull request. This is sort of a bore. Any chance you could trust some of us to have commit rights on the main branch, or .. something?

antirez commented 12 years ago

Helo seclorum, the way it works in all the github projects is this:

This way I can merge easily but at the same time I'm 100% sure about what gets in and what does not. I trust open source and open processes a lot but I think a central "authority" that accepts or rejects changes is needed.

When the fix is a simple oneliner, simply don't bother and just open a bug and I'll care to fix it myself. But this prevents you from receiving proper credits.

So it's not a matter of trust but the way I think good open source projects are done is by allowing contribution, by listening to the community, but with a single person (or for big projects a very small team) ultimately deciding about what to merge and what not.

Thanks for your help, Salvatore

ghost commented 12 years ago

Sorry for the 1-liner. I'm trying to learn git along the way.

seclorum commented 12 years ago

antirez: that works, I'll do that.

antirez commented 12 years ago

@cskyzi oneliners are good, it's a way to give credits to people discovering issues.

@seclorum thanks!

seclorum commented 12 years ago

btw, its what I've been doing, only through the github GUI, not at the cmd-line. ;)

seclorum commented 12 years ago

BTW, any chance we could make some progress with the args handling for -width/-height/-fullscreen? I would do it myself, but I have a feeling you have your own desires with regards to argv processing .. ;) If that can go in as a priority, I can then focus on fixing the key-handling for Pandora, which is slightly different than your average PC keyboard. Actually, it brings up a point: what to do about customizations for various target platforms? It'd sort of be nicer to have a way to configure things for a given platform at runtime, rather than at build-time - but whats your preference? I could submit a patch that uses #ifdef PLATFORM_PANDORA/#endif to block things out, or we could add args for "-platform <Pandora,BeagleBoard,rPi,etc>" .. whats your preference? I have a feeling that small tweaks like this will definitely be necessary for various platforms, alas.

seclorum commented 12 years ago

One other thing to consider: optimal compilation flags. On ARM we could definitely do some PLATFORM testing in the Makefile, for example, to get the right args needed for the target CPU ..

antirez commented 12 years ago

@seclorum --width and --height are implemented! --fullscreen, just a second and I'll do it. About PLATFORM testing, I agree this could be a good idea, at least if the gain in speed will be sensible.

antirez commented 12 years ago

p.s. just try ./load81 --help

seclorum commented 12 years ago

Great! I'll update the .PND to use --width and --height shortly.

antirez commented 12 years ago

Thanks, added --full as well, but does not work on osx (but I think it will on Linux).

seclorum commented 12 years ago

Okay I fixed picklelauncher to use the new cmd-line args and this is now how I will distribute the PND for OpenPandora users (without custom tweaks for the platform).

seclorum commented 12 years ago

Actually I just tested on OSX and --full works great for me - same with --height/--width too. Maybe it depends on which SDL you built with - the framework version, or brew, or ports, or whatever .. ;)