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 11241036:modular-plaza-system #42

Merged
merged 27 commits into from
Dec 29, 2024

Conversation

sebamarynissen
Copy link
Contributor

This PR adds 11241036's Modular Plaza System, as well as a bunch of its dependencies that weren't available in sc4pac yet. It provides variants for all of its 13 texture options.

A few things to note here:

  • It looks like 1 lot (MPS Square Column) is missing dependencies. It doesn't look like I have missed it, as I downloaded all of the listed dependencies separately and the props still didn't show up. I PM'ed 11241036 to see if he could verify.
  • It looks like the interactive sc4pac interface cannot handle 10 or more variant options as it doesn't allow you to select option 1 in this case - at least on Windows.
  • The rdg:prop-pack-1 was already included in @Zasco's sc4pac channel I chose to include it here to make it available on the default sc4pac channel as well, as that feels the natural thing to do with prop packs.

@memo33
Copy link
Owner

memo33 commented Nov 18, 2024

Oh my… More than 50 dependencies and still something's missing. I'm inclined to view it as the responsibility of the original author to list the complete dependencies on the STEX.

@sebamarynissen
Copy link
Contributor Author

Yeah, let me tell you, this one was a true pita to add. 😅 Nothing compared to the mattb325 packs. I've contacted 11241036 about the dependency issue, let's see what he has to say about it.

@sebamarynissen
Copy link
Contributor Author

sebamarynissen commented Nov 18, 2024

Another thing I was thinking of, the historic-harbor:dependencies package currently has twoo variants:

variants:
  - variant: { historic-harbor:dependencies:resolution: hd }
    assets:
      - assetId: historic-harbor-dependencies-hd
  - variant: { historic-harbor:dependencies:resolution: sd }
    assets:
    - assetId: historic-harbor-dependencies-sd

Would it make sense to provide a global variant for the preferred resolution? What should we call it in this case?

@memo33
Copy link
Owner

memo33 commented Nov 18, 2024

Would it make sense to provide a global variant for the preferred resolution? What should we call it in this case?

I think it's fine to keep it as a per-package variant, so that one can choose case by case. (In fact, I don't understand why this is an option at all. The BATter should have just chosen the more appropriate resolution for each individual prop or model.)

@sebamarynissen
Copy link
Contributor Author

I got a message back from 11241036 and turns out he indeed forgot to add a dependency. I'll update the package with it.

@sebamarynissen
Copy link
Contributor Author

Fyi: I've removed the petercrysti props again and moved them to #45. Hence #45 has to be merged now before this one can be merged.

@sebamarynissen
Copy link
Contributor Author

@memo33 ready for review

In particular, moves concrete walls from `jrj` to `buddybud` group.
@memo33
Copy link
Owner

memo33 commented Dec 29, 2024

Thanks a lot. Any chance the MPS dependency list gets shorter with your updated scripts? Otherwise, I'm happy to merge this as is. (I've made small changes, mainly renaming a few packages a bit.)

@sebamarynissen
Copy link
Contributor Author

I never used the tracking script for finding the dependencies as they were explicitly listed on the STEX. I did use the tracking script to find the missing dependencies though, but that has been fixed in the meantime. So no, unfortuntaly the dependency list won't get any shorter, but luckily we have sc4pac to handle those for us now.

@memo33
Copy link
Owner

memo33 commented Dec 29, 2024

Ah, I assumed you used the tracking script to resolve all the BSC Common Dependencies and Girafe Flora dependencies, as they are not listed on the STEX.

@sebamarynissen
Copy link
Contributor Author

Ah, could be. It's so long ago. Let me quickly run the script, I'm guessing a few can be shaved off indeed.

@sebamarynissen
Copy link
Contributor Author

Alright, I've removed the unneeded dependencies. I opened the city where I had plopped all the lots and no brown boxes appeared, indicating that the removal is ok.

@memo33 memo33 merged commit f4661f5 into memo33:main Dec 29, 2024
3 checks passed
@memo33
Copy link
Owner

memo33 commented Dec 29, 2024

Excellent. Thank you. Apologies for sitting on it for so long.

@Zasco With this PR merged, you could remove rdg:prop-pack-1 from your channel.

@sebamarynissen sebamarynissen deleted the feature/modular-plaza-system branch December 29, 2024 19:04
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.

2 participants