vegastrike / Vega-Strike-Engine-Source

Vega Strike Engine
Other
252 stars 44 forks source link

Refactor master part list #816

Closed royfalk closed 7 months ago

royfalk commented 7 months ago

This code decouple the MPL from rest of main body of code and make it and cargo part of resource lib. It makes it easier to test (some basic tests included) and also removes dependencies on the gfx folder among others.

This PR is part of a list of small PRs intended to come before #810. They are intended to clear the way for it and reduce the actual code in it to a manageable level. Note that #810 is a draft PR for review. Once these PR's are merged, I may break it up into smaller chunks for better processing.

Code Changes:

Issues:

stephengtuggy commented 7 months ago

Is this code compatible with PWCU and with Black Paralysis?

stephengtuggy commented 7 months ago

Is this code compatible with PWCU and with Black Paralysis?

You know what? I'm realizing that the game engine probably doesn't need to be compatible right this second. And that's an unrealistic expectation anyway. I would just like us to keep it in mind as something to get back to. Ideally before the release of 0.9.x.

Also, my apologies for not communicating more clearly, months ago, that we had lost compatibility with PWCU. It was something that I intended to keep up with, but then it kind of got away from me.

BenjamenMeyer commented 7 months ago

Don't worry too much about Black paralysis...I need to do a lot of work to bring that up to date with the engine.

We do need to give updates to PWCU to maintain compatibility though. That it one of our targets.

royfalk commented 7 months ago

Is this code compatible with PWCU and with Black Paralysis?

As a general rule, I leave compatibility concerns to the maintainers of the games themselves. That is, it is the responsibility of the game to be compliant with the engine and not vice versa.

Consider the reverse. We would leave carrier.cpp:587 to support a rather arbitrary decision for a specific game. Who were the hitchhikers. Why are they different from reactor_1 or cheese? Why should they deserve to be hardcoded into the engine?

Specifically however, I don't think I've changed anything that breaks WC here. I did break it before when I split the master parts list into several files and switched to JSON though.

Having said all of that, I intend to take a breather after lib_component and work on WC compatibility. I have a soft spot for that game and would like to replay it.

stephengtuggy commented 7 months ago

Is this code compatible with PWCU and with Black Paralysis?

As a general rule, I leave compatibility concerns to the maintainers of the games themselves. That is, it is the responsibility of the game to be compliant with the engine and not vice versa.

Consider the reverse. We would leave carrier.cpp:587 to support a rather arbitrary decision for a specific game. Who were the hitchhikers. Why are they different from reactor_1 or cheese? Why should they deserve to be hardcoded into the engine?

Specifically however, I don't think I've changed anything that breaks WC here. I did break it before when I split the master parts list into several files and switched to JSON though.

Having said all of that, I intend to take a breather after lib_component and work on WC compatibility. I have a soft spot for that game and would like to replay it.

Agreed. All of the above makes total sense. Thanks.

stephengtuggy commented 7 months ago

Do we still look for cargo with mission in the name? If so, I think I am good to approve this PR. Subject to the file conflicts being resolved, of course.

BenjamenMeyer commented 7 months ago

Is this code compatible with PWCU and with Black Paralysis?

As a general rule, I leave compatibility concerns to the maintainers of the games themselves. That is, it is the responsibility of the game to be compliant with the engine and not vice versa.

Generally I agree; though we should have a core set of games that we help maintain when we break something. Obviously we cannot support every one; but for the moment I'd define the cor te set as VS:UtCS and PWCU; and eventually adding VSU:BP to that. Aside from any small test game assets we develop for purpose of specific testing I don't see the group going beyond that. I'm mostly looking at us helping to fix breaking changes when they happen on those asset sets, not general contributions. Hopefully other game developers will be able to monitor the engine and those asset sets to be able to know what's going on and about breaking changes.

NOTE: I include PWCU in that list since it's pretty much just @ermo that consistently maintains it with elpinsome contributions from us/me.

Consider the reverse. We would leave carrier.cpp:587 to support a rather arbitrary decision for a specific game. Who were the hitchhikers. Why are they different from reactor_1 or cheese? Why should they deserve to be hardcoded into the engine?

Certainly agree that we should not be providing hard coded stuff in the engine for specific game assets; anything we do in the engine should be for all game assets.

Specifically however, I don't think I've changed anything that breaks WC here. I did break it before when I split the master parts list into several files and switched to JSON though.

There's been very little we've done as breaking changes thus far. We'll probably have more to come. At some point we need to heavily document and standardize the API for the assets; most of it right now should be minor and easy fixes.

Having said all of that, I intend to take a breather after lib_component and work on WC compatibility. I have a soft spot for that game and would like to replay it.

:+1: I'm going to try to help get 0.9.x out but also want to focus on getting VSU:BP back up and running too. That'll teach me a lot about the game asset side in the process as well. I can already tell there's some asset data that should probably be provided by the engine since it'll be common for all assets since there's things I'm going to have to copy from VS:UtCS to VSU:BP to get it working again.

royfalk commented 7 months ago

Something broke in the commits. It caused git to automerge badly. I'm closing this and starting fresh.