SplitScreen-Me / splitscreenme-nucleus

Nucleus Co-op is an application that starts multiple instances of a game for split-screen multiplayer gaming!
https://www.splitscreen.me/docs/what-is-splitscreen-me
GNU General Public License v3.0
856 stars 50 forks source link

CONTRIBUTING.md (+README.md edits +CODE_OF_CONDUCT.md) #78

Closed RangerTJ closed 2 months ago

RangerTJ commented 3 months ago

Goal: Addressing Issue #22

Drafted a Contribution Guide with the intent of helping to consolidate links to existing documentation into a hub that has centralized guidelines / tips to get new contributors started.

My goal was that, by the end of reading the guide, a new user would be able to:

While I think I covered the essentials pretty thoroughly, based on scouring existing documentation and my chats with Mikou27, there may very well be gaps/some content issues that need to be addressed. If so, let me know, and I'll get them fixed up and update the PR ASAP!

Notably, the style guide is a bit barebones, though I'm not sure if the project is actively looking to enforce any particular standard at the moment, which is why I mostly focused on C# "common standards" and high-level guidelines to promote code readability.

(Also, if this gets cross-threaded with the timing of any incoming updates, I can update my fork and let everyone know when the PR is ready to be reconsidered).

RangerTJ commented 3 months ago

Just caught a bug I'll need to fix - the readme links to the wrong repo's version of CONTRIBUTING.md.

RangerTJ commented 3 months ago

Swapped the readme link to a relative path to CONTRIBUTING.md, so it should be good to go now.

RangerTJ commented 3 months ago

This is good and all but it's a bit long. Lots of what is written here should be in a CODE_OF_CONDUCT.md file instead.

I'd wish there was more technical information, such as how to start the development server, dependencies, technologies used, etc...

Perhaps @Mikou27 could help with the technical part?

I'm moving my earlier reply to this comment for better visibility, since implementing the readme marked the conversation as resolved.


Thanks for the feedback!

I hadn't originally planned on tackling the Code of Conduct to try and limit scope, but... that said, I could definitely move the more "behavioral"/soft stuff to a code of conduct doc, if that'd be a helpful thing to add (though not sure if I should do that as a separate PR).

If I did do that, is there anything specific that you think should be moved to that doc / anything else that you think a code of conduct doc would need (other than the more traditional "don't be a jerk" type of stuff)? My initial thought is to move the whole "Communication" section there and then patch in the gaps for an initial Code of Conduct.

In terms of technical dependencies, were you thinking stuff like the version of .net that needs to be installed + a brief bit on what each of the submodule dependencies does? Or something a bit more detailed on them?

As I think about it, if the guide is starting to get a bit long too as it is, and it'd be good to keep it briefer, do you think it'd make sense to move relevant stuff to a Code of Conduct + maybe link to a more detailed article(s) about the dependencies and create that as a seperate issue to tackle? And then edit in a link to it from the contributor guide when that one is done? Depending on how much detail is needed, I could see that ballooning things quite a bit.

I wonder if a link to the detailed history doc from the SplitScreen.Me site might be helpful in here too? Maybe as a "read this first to know the development history of what you will be working on" kinda thing?

Thanks again! This is all really good stuff! Like I didn't even know there was a development server to work with, so that's exactly the kind of content gap that I was looking to identify.

Snailedlt commented 3 months ago

@RangerTJ first of all, thank you for contributing and caring so much about getting it right, that is excellent!

Feel free to disagree or ask more about what I'm about to say :)

In my experience the CONTRIBUTING.md file usually contains a few sections:

As for the code of conduct, I'd recommend looking at the CODE_OF_CONDUCT.md file in this repo: https://github.com/devicons/devicon

We want it to be less general than that though. We especially want to make sure to include that we don't want to be associated with or included in illegal activities. For example we should include stuff like this in the file:

Hope that answered some of it. Feel free to ask more questions if you have any 😊

Mikou27 commented 3 months ago

This is good and all but it's a bit long. Lots of what is written here should be in a CODE_OF_CONDUCT.md file instead. I'd wish there was more technical information, such as how to start the development server, dependencies, technologies used, etc... Perhaps @Mikou27 could help with the technical part?

I'm moving my earlier reply to this comment for better visibility, since implementing the readme marked the conversation as resolved.

Thanks for the feedback!

I hadn't originally planned on tackling the Code of Conduct to try and limit scope, but... that said, I could definitely move the more "behavioral"/soft stuff to a code of conduct doc, if that'd be a helpful thing to add (though not sure if I should do that as a separate PR).

If I did do that, is there anything specific that you think should be moved to that doc / anything else that you think a code of conduct doc would need (other than the more traditional "don't be a jerk" type of stuff)? My initial thought is to move the whole "Communication" section there and then patch in the gaps for an initial Code of Conduct.

In terms of technical dependencies, were you thinking stuff like the version of .net that needs to be installed + a brief bit on what each of the submodule dependencies does? Or something a bit more detailed on them?

As I think about it, if the guide is starting to get a bit long too as it is, and it'd be good to keep it briefer, do you think it'd make sense to move relevant stuff to a Code of Conduct + maybe link to a more detailed article(s) about the dependencies and create that as a seperate issue to tackle? And then edit in a link to it from the contributor guide when that one is done? Depending on how much detail is needed, I could see that ballooning things quite a bit.

I wonder if a link to the detailed history doc from the SplitScreen.Me site might be helpful in here too? Maybe as a "read this first to know the development history of what you will be working on" kinda thing?

Thanks again! This is all really good stuff! Like I didn't even know there was a development server to work with, so that's exactly the kind of content gap that I was looking to identify.

Hi man, imo you should continue on your path and improve over time (if you feel it) do it little but do it 😉. Ideas and suggestions are good and all but don't get the job done. Do not hesitate to dm me if needed.

RangerTJ commented 3 months ago

@Snailedlt Thanks! That's super helpful!

Let me see if I can break down some of my thoughts and just make sure I'm understanding correctly, so I can create some more actionable chunks to work with.

How to contribute

It sounds like I could largely leave this as-is, other than re-arranging some things and moving more "soft skill" chunks of it to the Code of Conduct.

How to report a Bug or suggest a feature

Would you say that the current version of this is over-written, and could basically be simplified to "use the issue form and follow the template" with a link to the existing issue article in the FAQ (to keep it more concise), along with the explicit "don't make issues about these things" list? Maybe with a link to the code of conduct with a mention of contribution etiquette once the conduct doc exists?

How to develop locally

I was originally thinking that this was handled by site compiling guide (since this had the git command to clone the repo with submodule dependencies), but it now occurs to me that compiling and setting up a dev environment are technically two different things, heh (even if compiling more or less requires setting up a dev environment). I was able to build successfully, so I initially assumed the compile guide might have been adequate, but... it sounds like it might not be, lol. Also, now that I look at it, that link mentions cloning the zerofox fork instead of that one, so that's another thing I'd need to address, even if it just stayed a link (unless we're supposed to clone from that fork for some reason?).

Do you think it would make sense to go granular in terms of having setup instructions for a recommended IDE, or just focusing on the git commands (and stuff like .net devkit requirements) needed to successfully clone the repository so that it's ready for work?

I apologize for the quasi stream-of-consciousness here as I kinda outline how to tackle this in my head, lol.

Do you think something like the following would be sufficient for the "Local Development" section?

Tech Stack

Local Setup

Building Nucleus Coop

How to Submit

Release Builds

RangerTJ commented 2 months ago

Just finished a first pass at updating the doc by expanding the setup guide. Note that I specifically have not yet given the document a simplification pass (so as of right now, it's even longer, lol), so expect trimming of some of the stuff that could be explained by Github articles or could be passed along to existing SplitScreen.Me ones (such as the bug report guide), along with a bunch of stuff moving to an "etiquette tips" part of a Code of Conduct doc. Also added a bit about NO PIRATES to the communication section to make sure I don't forget that, lol.

I think there might be some technical issues with some of the setup instructions, but those are things that I think I'll need to DM Mikou27 about. Theoretically they should be good though (assuming the other contribution guide is accurate), there just may be some missing bits to sort out, like compatibility with VS2022, etc. So I think it should be more about adding a little extra version requirement info vs. totally re-writing anything.

RangerTJ commented 2 months ago

I went ahead and threw together a 1st draft of a code of conduct based on the template you linked. For now it's largely the same, though I added an explicitly-headed section that put the anti-piracy stance front and center, along with migrating some of the etiquette points I'd drafted earlier from CONTRIBUTING.md (though some of those may still need to be shrunk a bit). I also added a link to the Discord channel rules... Which I could probably put in the doc itself, if that made more sense (though not sure if the Discord rules are just specific to discord or not).

There's currently a potential placeholder in the portion that covers who to report behavior violations to. In the template/devicons example it looks like there's a dedicated "reporting stuff" email, but I'm not sure if Nucleus Coop has the same? For now I just said to go to the support channel and ping the mods/admins as a stopgap measure, but not entirely sure if there's a preferred approach (or if it should go into more detail if that stays the approach, like ping to mention here's an issue, then mods will ask for details in DM's to protect privacy of reporting or some such... this would also probably apply to the security issues part of CONTRIBUTING.md, I'd imagine).

Note that I haven't yet culled the stuff I moved over from CONTRIBUTING.md yet.

RangerTJ commented 2 months ago

Trimmed the stuff I moved to the code of conduct from CONTRIBUTING.md + some minor code of conduct edits. Added a bit to code of conduct about avoiding unsolicited DM's, and the best way to reach people until you are asked/given the OK to talk to someone in DM's.

RangerTJ commented 2 months ago

Just wanted to add a quick note - I chatted with Mikou27 last night/this morning and figured out my issue with compiling ProtoInput was a missing dependency for it. I'll see if I can track down what it is and add it to the pre-req part of the doc.

RangerTJ commented 2 months ago

Finished incorporating most of the recent suggestions (and also synch'd my fork with the master repo, since it looks like it had been 1 commit behind... ironically due to the meta form PR I submitted, lol).

I think there's a few notable points to work on more before I think this PR is fully ready for a "final" review:

RangerTJ commented 2 months ago

How long do you think this will take, and do you need any help with it? If it takes a lot of time, we can merge this PR, and iterate on it afterwards. What do you think?

Short version is that I'd be fine with merging the PR and iterating later, since I figure this still gives a reasonable starting point, even if there's some known gaps (and I suspect some of the gaps, like compiling Proto Input, are going to always have a certain degree of system-specific troubleshooting involved that may be hard to capture in a general doc).

Long Version I've attached some details of what I've managed to figure out so far, so if anyone has ideas on how to bridge the gaps, I'm totally open to any help on that front. If anyone can get Proto Input and the Nucleus coop to compile and run already without issue, it'd also be super helpful to get a snapshot of what dependencies are installed for Visual Studio / Builder, so I could potentially narrow things down (or at least use them as a frame of reference for a successful compile).

It's taking a bit longer than I expected to get the compiling for Proto Input sorted out, since there's so many potential dependencies, and no docs about it on the Proto Input repo itself. I was hoping it'd just take a day or two but... now I dunno, lol. It's been a bit of random chance as to whether any given dependency changes anything, and there's still the wildcard of a potentially different dev environment than what the original docs were written for being a factor. I have had a little luck today, but I still appear to have one (or more) missing dependencies. Mikou27 mentioned a trial-and-error approach to getting it to work that required so many dependency installs that it's hard to say what the exact requirements are, unfortunately. It sounds like it compiles fine if you basically install every potential dependency, but... there's a lot of them, lol. That said, there's also a known work-around that could be used in the interim, or as the default path (with the caveat that I'd like to double-check with Mikou27 that I'm not missing any other dependencies/files with the current work-around approach).

What I'm inclined to do is modify the compiling instructions for Proto Input to be a branched path, depending on whether you want to work with Proto Input code (which is debatably outside the scope of Nucleus Coop itself anyways).

Workaround Details A workaround exists, in that you can just grab the post-compiled Proto Input files from the most recent Nucleus Coop release, drop those in the root folder, and Nucleus Coop should compile/run fine after that. Mikou27 had mentioned that if you don't plan on working with Proto Input, that's a viable approach. Given contributors to Nucleus Coop are probably going to focus on Nucleus rather than Proto Input, it might even make sense to encourage grabbing the required files from the latest release build as the suggested path (since I'd imagine working with Proto Input would be done through the Proto Input repo anyways).

I've committed a proposed change that has the work-around as the "default" path, if you want to take a look at that. If this seems a bit messy since it's still got some uncertain elements as well, we could potentially just go back to just saying to compile ProtoInput instructions at a high level and mention they may need to ping the developer channel if they run into any issues, or something along those lines (and maybe move the work-around to the bottom section with troubleshooting, etc).

Given how messy the compiling Proto Input is becoming, the more I poke around with it, maybe using the workaround as the suggested starter path makes sense? I've gotten it to where the only compile errors are "FILE_INFO_BY_HANDLE_CLASS" undefined syntax errors, after installing ATL build tools dependencies and manually editing ProtoInput.sln to fix some stuff that appeared to be compiling out of order. I'm having a rougher time trying to find the source of the syntax error, unfortunately. I'm reasonably sure I have the theoretical dependency installed, at least on paper, so it might be a version issue possibly? Or some permutation of settings/dependencies that results in things installing out of order / triggering syntax errors? Installing everything C++ takes up a lot of space, so I'm kinda hesitant to do that, lol. Assuming the FILE_INFO_BY_HANDLE_CLASS solution fixes the last of the remaining compiling errors, that's the only blocker left to fully documenting how to successfully compile Proto Input (though this assumes there isn't a new error after addressing this, lol).

I've drafted a version of the doc that presents both options, for the time being, so maybe this is a good starting point that can be updated later, and conversation in the Dev channel can fill in the gaps in the meantime?

@Mikou27 - Question about the Workaround Approach I noticed that, upon closer inspection, compiling the current build from the master branch relative to the last release results in a few less things showing up in the "utils" folder when I batch build, so I'm not sure if those are things that would have been built from Proto Input? Or just stuff manually added for the release itself? I'm not sure if this is an issue, or normal behavior, in case you have any thoughts on it.

Specifically, the following folders under "util" are all in the release build, but I'm not seeing them when I build from the Nucleus Coop .sln file + drop in the missing DLL's from Proto Input:

I got some crashing behavior once the 2nd game window opened, but it looks like that was just due to the backup folder not existing in "utils" at the time. Creating the folder in release\utils\ directory stopped the crash, so I'm not sure if these folder not showing up in "utils" is still an issue or not. I see them under \splitscreenme-nucleus\Master\NucleusGaming\Tools\, so it seems like it would make sense for them to be built, but it doesn't look like these are actually related to the Proto Input compile issue (since they seem to be things covered under the Nucleus.Gaming tools section). It looks like I might be having a separate issue with Nucleus Coop itself compiling, if these should compile with the rest of the program, but don't. Unless it's a trickle-down effect of Proto Input files not starting off where expected maybe? I noticed lack of Proto Input didn't throw any errors when compiling Nucleus Coop, so maybe it's stealth trickle-down effects from that?

Assuming the above folders in "utils" aren't related to Proto Input, we could probably suggest the "workaround approach" without issue, I think. (Long term it would definitely be good to document how to successfully compile Proto Input so there's better continuity of knowledge on the topic, but that could be addressed separately).

RangerTJ commented 2 months ago

Update: Thanks to help from Mikou27, I got Proto Input to compile! It was kind of a messy process, which was likely related to loading it in VS2022 (and I think some of my earlier issues were from upgrading the project to v143 rather than telling it not to), so I think I may distill it down to a "here's things to check" / be aware of, since it seems very sensitive to your dev environment.

RangerTJ commented 2 months ago

Just implemented the additions to the Proto Input compile article and put a good bit about troubleshooting compiling it down in the tips section. I did verify that this didn't make a difference in the folder discrepancy mentioned above, so I think we should be good leaving the work-around as a suggested approach for people that don't plan to mess with proto input source code.

If it turns out that the compile instructions for Nucleus Coop are good, I think all that's left is just a final polish pass on the most recent round of edits. I'm working with Mikou27 regarding release/master branch compile discrepancy in the utils folder. I will update here once I know whether there's a gap I need to address (and if there is, we'll want to update the compile guide for nucleus to include any missing bits).

Update: I think the discrepancy might just be due to the current version of NucleusCoop.sln file on the repo being out of sync with the actual "latest" version used for development (since the repo version that I cloned from shows it was last updated ~2 years ago), so I don't think this is actually an issue with the compiling steps, just a result of compiling with an older .sln file.

RangerTJ commented 2 months ago

Added a final(?) round of polish edits based on the last round of feedback for a few things that I thought would help beyond the suggested commits. Most of the additional changes were slight alterations that I thought might help with language clarity a bit. Mostly it was adapting the "caution" suggestion tone to the "important" suggestion version for the compile path note, and breaking a couple long sentences into multiple sentences. This should hopefully help with readability.

I also took the final product of those edits and then ran it through MS word to do a spelling/grammar check, and caught a few more things that slipped through the cracks.

Snailedlt commented 2 months ago

@Mikou27 Please look over the technical details again to make sure it still looks good :)

Mikou27 commented 2 months ago

Awesome work! Thank you very much for this huge contribution to the project!

Snailedlt commented 2 months ago

Yes, great work @RangerTJ!