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

Folder restructuring from primedev #624

Merged
merged 13 commits into from
Dec 27, 2023

Conversation

ASpoonPlaysGames
Copy link
Contributor

Has to cherry pick by hand because github is dumb but yeah

Cherry picking ccf5772

@ASpoonPlaysGames
Copy link
Contributor Author

Completely untested btw, so yeah

@ASpoonPlaysGames ASpoonPlaysGames changed the title Folder restructuring from primdev Folder restructuring from primedev Dec 21, 2023
@ASpoonPlaysGames ASpoonPlaysGames added needs testing Changes from the PR still need to be tested needs code review Changes from PR still need to be reviewed in code labels Dec 21, 2023
@GeckoEidechse
Copy link
Member

I have some concerns with renaming NorthstarDLL to primedev as to both existing and new contributors, the folder rename might add extra confusion.

That being said, I'm okay with forgoing those concerns by accepting the change on the short-term in order to more easily keep merging changes from primedev.

I'd love to also here the input from people who currently have PRs open, as they will be affected by this in the shape of merge conflicts. In particular I'd love some input from:

@uniboi
Copy link
Contributor

uniboi commented Dec 21, 2023

#613 adds files so it should be merged after this pr

@F1F7Y
Copy link
Member

F1F7Y commented Dec 21, 2023

I dont think this is the correct path to take.

You'll get very little benefit from just moving files around without modifying their contents.

@uniboi
Copy link
Contributor

uniboi commented Dec 21, 2023

cherry picking more is going to be a pain without this commit though

@F1F7Y
Copy link
Member

F1F7Y commented Dec 21, 2023

cherry picking more is going to be a pain without this commit though

I never said you should cherry pick differently or that you should cherry pick at all. I also never said primedev was finished or more stable.

@GeckoEidechse
Copy link
Member

GeckoEidechse commented Dec 21, 2023

@F1F7Y, I thought the idea was to carry over logical changes from primedev into Northstar in order to get the good parts of primedev in Northstar?

What would be your suggestion in order to get (parts of) primedev in Northstar? ^^

@F1F7Y
Copy link
Member

F1F7Y commented Dec 21, 2023

@F1F7Y, I thought the idea was to carry over logical changes from primedev into Northstar in order to get the good parts of primedev in Northstar?

What would be your suggestion in order to get (parts of) primedev in Northstar? ^^

My suggestion would be to rewrite Northstar and release a 2.0 version. This obviously requires talent and time Northstar doesn't have currently.

Northstar Prime is me giving up on Northstar but not wanting everything i learnt go to waste. At the time the logical solution i saw was to make a fork and work on it without having to worry about PRs or having any broad discussions. After a few months of work i got burnt out, hence why I don't recommend copy pasting primedev as there's a bunch of temporary solutions that were done to make it easier to restructure entire systems.

@GeckoEidechse
Copy link
Member

Hmmm, aight. Any suggestions what you'd change in Northstar for 2.0 refactor or the like? Maybe we can get those worked on and figured out together.

Feel free to drop them here, in a new Github issue/discussion, or just directly in my DMs. Whatever works best <3

@F1F7Y
Copy link
Member

F1F7Y commented Dec 21, 2023

Hmmm, aight. Any suggestions what you'd change in Northstar for 2.0 refactor or the like? Maybe we can get those worked on and figured out together.

Feel free to drop them here, in a new Github issue/discussion, or just directly in my DMs. Whatever works best <3

Most things others have already been saying for months. Better plugin system, better mod system, more developer focused features such as debug navmesh rendering, etc...

@GeckoEidechse
Copy link
Member

Most things others have already been saying for months. Better plugin system, better mod system, more developer focused features such as debug navmesh rendering, etc...

Yeah, unfortunately, in the information blackhole that is Discord, stuff gets lost quite easily, hence I tend to push for more discussion on GitHub.

That being said:

Copy link
Member

@GeckoEidechse GeckoEidechse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the move to subdir but not so much the moving of launcher into subdir of DLL as to my knowledge the two are not that tightly coupled.

That being said, I'm aware that a temporary change to mimic primedev will help a lot in getting stuff moved over.

Proper code review + test tbd.

@ASpoonPlaysGames
Copy link
Contributor Author

I like the move to subdir but not so much the moving of launcher into subdir of DLL as to my knowledge the two are not that tightly coupled.

IMO it would be a bit dumb to have DLL in its own subfolder within the base folder, because like 95% of the codebase is DLL and having to go an extra folder deep to access the vast majority of things is not great.

(also i believe primedev has them a bit more tightly coupled later down the line, with launcher doing the logging and stuff or something)

Copy link
Member

@GeckoEidechse GeckoEidechse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, from looking at far as I can tell it's all just the file move as expected.
(I already voiced my opinion on the change so I'm not gonna repeat it again)

As for testing, there's not much to test as it's just a file move so if it compiles fine it should just work™
(I still launched client once just in case lol)

@GeckoEidechse
Copy link
Member

Already sorry in advance to everyone having a PR open rn ^^"

@GeckoEidechse GeckoEidechse merged commit f5ab6fb into R2Northstar:main Dec 27, 2023
2 checks passed
uniboi pushed a commit to uniboi/NorthstarLauncher that referenced this pull request Jan 2, 2024
Copies of over the primedev folder structure for easier cherry-picking of further changes

Co-authored-by: F1F7Y <filip.bartos07@proton.me>
@ASpoonPlaysGames ASpoonPlaysGames deleted the primedev/one-folder branch January 4, 2024 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs code review Changes from PR still need to be reviewed in code needs testing Changes from the PR still need to be tested
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants