AmbientRun / afps

A moddable FPS game written with the Ambient engine
6 stars 3 forks source link

Add "laser box" entity to afps #4

Closed droqen closed 1 year ago

droqen commented 1 year ago

hello! please feel free to reject this PR, i'd just never done it before and it was like a one-button thing in VS Code! i'm lacking a piece of functionality, my lasers should kill but i can't get them raycasting properly.

droqen commented 1 year ago

it also adds my ridiculous godot scene loader, which spawns a huge non-colliding cube to the middle of the map, so it's probably not a good PR to accept. at all

chaosprint commented 1 year ago

That's a good start. Two things to change:

Feel free to ask on Discord for details

droqen commented 1 year ago

Title remedied!

Can you confirm my understanding of how what I did differed from the first bullet point? Here's what I think I did:

And here's what I think you're telling me to do instead, just one step difference?

What do I do with my fork afterwards? Do I just forever have my own fork from that point onward and need to keep it synced it up with origin?

If I'm totally wrong lmk and I'll ask you for some help on Discord when I'm free, haha

ten3roberts commented 1 year ago

What I do is:

I do the draft state to let others know that Hey, this feature is being developed so that we don't end up with multiple people doing the same thing. This is more common in open source where are distributed and don't necessarily know each other or have means to communicate that well.

I usually skip the creating branch on my fork step as it is easier to find the difference woth my fork and the upstream as any people who find it won't have to find a the branch in the dropdown containing my changes.

Throughout the process I maintain a discussion about the feature either in the draft state PR or beforehand in an open issue

If there are lots of changes from upstream I either rebase my changes ontop or 3-way merge them in.

ten3roberts commented 1 year ago

Example:

This is a fork of ambient, main has no changes from ours, and I don't know which of the 2 branches are the one that I'll find the changes in if I would find this fork by just passing by.

image

... which is why I prefer to keep the changes on main, unless I am working on multiple features at the same time (which is usually an indication that the fork is too large or that the upstream repo is abandonded, so that makes the fork serve a different purpose than "making a feature and merging it in")

ten3roberts commented 1 year ago

Another pro tip of github proficiency.

If you open a PR where the branch you want to merge contains just one commit, the title and description will be automatically filled by the message of the commit. Makes it consistent where you write a bugfix commit and explain the bug in a comment such that the PR description has the same explanation