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

Fixed #893 VenomShooter Ammo #958

Merged
merged 2 commits into from
May 17, 2012
Merged

Fixed #893 VenomShooter Ammo #958

merged 2 commits into from
May 17, 2012

Conversation

0x0ade
Copy link
Contributor

@0x0ade 0x0ade commented Apr 15, 2012

  • Added check if the owner is player or not , if it's player use valid y position

@master-lincoln
Copy link
Collaborator

I would merge the venom fix right now. But I don't know if we should accept more tiles now...

@0x0ade
Copy link
Contributor Author

0x0ade commented Apr 15, 2012

I understand . Removing as soon as I removed my Joypad Implementation temporarily ...

@master-lincoln
Copy link
Collaborator

You should really try to use a branch for each feature or at least each Pull Request.
We have some basic info on that in the wiki

@0x0ade
Copy link
Contributor Author

0x0ade commented Apr 16, 2012

Sorry , I can not remove the tile as for now ( IRC - My Joypad Implementation ... just got an " Milestone " : Bindings are working for Buttons , gonna add Axes ) . I may resend this pull request after finishing the joypad implementation if nobody has merged it to there .

Usually can't you manually merge the fix from the diff ( thru your fork ) ? That would be even easier and faster , or must someone pull request it ? I think I am missing some limits ...

EDIT : Removing everything - then just updating this pull request ... almost finished with joypad implementation , just precise shooting / mouse movement needs work ...

@0x0ade
Copy link
Contributor Author

0x0ade commented Apr 16, 2012

No comment - Git just changed the IDs of some files and hasn't committed changes -.-'' Gonna retry to fix issue ASAP on current base of this repo =.=

Such ... an ... messup ... I think next time I will really use branches for functions ... I have found parts of the Joypad in the diff ... hopefully the current upload ( forced an modified , redownloaded version of this repo ) will not fail

@0x0ade
Copy link
Contributor Author

0x0ade commented Apr 16, 2012

WOOP It worked , proper commits committed , now mergeable

@0x0ade
Copy link
Contributor Author

0x0ade commented Apr 17, 2012

@kylebrodie facepalm fuuuuuu

But wait - won't pos.set(e.pos.x + xa * 4, e.pos.y + ya * 4); in Bullet still work ?

@kylecbrodie
Copy link
Contributor

Oh yes I see. It isn't necessary. You could change: bullet.pos.y = bullet.pos.y-19; -> bullet.pos.y -= 19; to make it a little more readable.

@0x0ade
Copy link
Contributor Author

0x0ade commented Apr 18, 2012

Could ... won't do it because I will now kinda prepare myself for Ludum Dare . Even if : I would have to re-download the developer branch which takes ( by magic 150 kb/s where 1600 is my max ... ) 30 minutes .

@kylecbrodie
Copy link
Contributor

I could do it and then send you a pull request and it would be added into this pull request.

Flet added a commit that referenced this pull request May 17, 2012
@Flet Flet merged commit 91701f4 into Maescool:develop May 17, 2012
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.

4 participants