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

Add truck stops #48

Merged
merged 32 commits into from
Dec 29, 2024
Merged

Conversation

sebamarynissen
Copy link
Contributor

This PR adds a bunch of truck stops.

I've had to split up a lot of packages in resources and lot parts because the American Truck Stop has an insane amount of dependencies. I was thinking that maybe this kind deserves a separate subfolder category, like 110-shared-resources or something because 100-props-textures doesn't cover the purpose. For now I've put all these resource packages under 100-props-textures.

This PR also relies on a few dependencies that @Zasco has added in his own channel. I'll let the GitHub action fail and have it find out which ones I still need to copy over.

@sebamarynissen
Copy link
Contributor Author

Apparently the only dependency missing was w-swietwoot:dutch-prop-pack.

@Zasco
Copy link
Contributor

Zasco commented Nov 25, 2024

This PR also relies on a few dependencies that @Zasco has added in his own channel. I'll let the GitHub action fail and have it find out which ones I still need to copy over.

I'm not sure having the default channel depend on other channels is a good idea... It is expected to remain maintained as long as sc4pac is, but other channels have no garantee on that. The default channel should be complete in itself.

@sebamarynissen
Copy link
Contributor Author

@Zasco Yes I agree, that's why I added the missing package in this PR as well. Once it gets merged, you can deleted it from your channel.

@sebamarynissen

This comment was marked as off-topic.

@memo33

This comment was marked as off-topic.

@memo33
Copy link
Owner

memo33 commented Nov 25, 2024

By the way, I'm going to try to review your PRs in a few days.

@sebamarynissen
Copy link
Contributor Author

sebamarynissen commented Nov 25, 2024

Take your time, there's no rush from my side. Just know that my scraping scripts are now in a state that very little manual intervention is still needed, even for the descriptions and dependencies, so you'll probably see some more PRs coming in from me in the following days. :)

Btw, I'll create a separate issue to discuss the exchange-specific channels. If you could move your comment to that issue, then I'll reply to it there. Keeps this PR clean.

Copy link
Contributor

Choose a reason for hiding this comment

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

@sebamarynissen thoughts about changing w-swietwoot:dutch-prop-packbnl:dutch-prop-pack as it was released under the BNL team banner?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@noah-severyn That makes sense I guess. It's not always clear to me whether a package should be put under the author's name or under the team name. Do you have any guidelines for this @memo33 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

@noah-severyn That makes sense I guess. It's not always clear to me whether a package should be put under the author's name or under the team name. Do you have any guidelines for this @memo33 ?

I have a preference for the team name if either the title, images, or description makes reference to the team name. IMO it's a good way to group similar content as groups tend to have a "theme" so it could make a quick thing to search for if you find one piece of content you like. Plus, I just personally kind of enjoy the history. :P

Copy link
Owner

Choose a reason for hiding this comment

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

Do you have any guidelines for this @memo33 ?

Not really. I mostly go by intuition, what's more recognizable and what's used in other uploads. In the end, it's a question of what makes the package easy to find or identify. To that end, if an author name or some essential acronym is not part of the group or package name, it can be included in the summary, so it can be found via search.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Allright, I'll put the dutch prop pack under bnl flag then. I guess especially for prop packs it makes sense to release them under the team name, as props released under a team name are probably used by various batters of that team.

@sebamarynissen
Copy link
Contributor Author

@memo33 ready for review

@memo33
Copy link
Owner

memo33 commented Dec 29, 2024

Thanks. I chose to merge the two RHD/LHD Esso station packages into a single package using driveside variants. Although, they can technically be used together, packaging it this way is more intuitive. I think most people won't miss the opposite driveside variant, as the NAM doesn't have FA-RHW inside ramps that would help with constructing "in the median" setups.

I was thinking that maybe this kind deserves a separate subfolder category, like 110-shared-resources or something because 100-props-textures doesn't cover the purpose. For now I've put all these resource packages under 100-props-textures.

I'm open to this idea. For now, using 100-props-textures should be fine. The main question we need to ask ourselves is whether this distinction is understandable and useful for end users.

@memo33 memo33 merged commit 8b4848e into memo33:main Dec 29, 2024
3 checks passed
@sebamarynissen sebamarynissen deleted the feature/american-truck-stop branch December 29, 2024 15:18
@sebamarynissen
Copy link
Contributor Author

Definitely makes sense indeed if the NAM doesn't have those ramps available.

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