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

Split vanillla object from levels allowing pruning vanilla assets on fast64 imports and managing vanilla objects independently of levels #796

Open
wants to merge 6 commits into
base: develop/3.0.0
Choose a base branch
from

Conversation

aglab2
Copy link
Collaborator

@aglab2 aglab2 commented May 1, 2024

This PR introduces a new bank for vanilla objects split from the courses. This allows for the following nice improvements:

  1. Vanilla objects can be used in multiple courses. For example it is common that binary hacks use TTC objects in multiple courses at once.

From level script it looks like

LOAD_VANILLA_OBJECTS(bob, generic),
...
JUMP_LINK(script_func_vo_bob),
  1. Vanilla courses were setup in such a way that fast64 imports prune most of the useless vanilla leftovers. Level geolayouts/collisions/models were moved in *.inc.c that fast64 overwrites.

Unfortunately, this PR is very gigantic and I am not sure I can do much about reducing its complextity but I would really appreciate if I can get this merged to make handling vanilla stuff significantly more convenient.

@aglab2 aglab2 requested a review from gheskett as a code owner May 1, 2024 15:45
@gheskett
Copy link
Collaborator

gheskett commented May 1, 2024

Important question, does this screw over 4MB users?

@aglab2
Copy link
Collaborator Author

aglab2 commented May 1, 2024

Nope, in fact it should help 4MB users because levels will not have garbage in them with fast64 imports.

@gheskett gheskett added enhancement New feature or request needs verification Needs to be tested to ensure functionality high priority Important, non-critical issue or feature / high priority monkaS monkaS labels May 1, 2024
@gheskett
Copy link
Collaborator

gheskett commented May 1, 2024

Can you rebase this onto develop/3.0.0 instead of 2.3? This is very much major release territory.

@gheskett gheskett added this to the 3.0 milestone May 1, 2024
@gheskett gheskett requested review from arthurtilly and Arceveti May 1, 2024 16:27
@aglab2 aglab2 changed the base branch from develop/2.3.0 to develop/3.0.0 May 2, 2024 00:49
@aglab2
Copy link
Collaborator Author

aglab2 commented May 2, 2024

Rebased to develop/3.0.0. Original 2.3.0 commit is available at https://github.com/aglab2/HackerSM64/commits/split-levels-2.3.0-squash/

Copy link
Collaborator

@gheskett gheskett left a comment

Choose a reason for hiding this comment

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

Good grief, GitHub is really on the struggle bus with this one; I straight up had to skip the last entire quarter of files because GitHub stopped pre-loading diffs, and waiting for 200 them to load would literally take me all night.

.gitignore Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
actors_vanilla/ssl/header.h Outdated Show resolved Hide resolved
assets.json Outdated Show resolved Hide resolved
data/behavior_data.c Outdated Show resolved Hide resolved
levels/bitdw/areas/script_vanilla_load.inc.c Outdated Show resolved Hide resolved
levels/bitfs/areas/script_vanilla.inc.c Outdated Show resolved Hide resolved
levels/bitfs/areas/script_vanilla_load.inc.c Outdated Show resolved Hide resolved
levels/bob/geo.inc.c Outdated Show resolved Hide resolved
src/game/behaviors/bowling_ball.inc.c Show resolved Hide resolved
actors/group5_geo.c Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
Copy link
Collaborator

@gheskett gheskett left a comment

Choose a reason for hiding this comment

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

Send it :monkaS:

@aglab2 aglab2 requested a review from arthurtilly May 3, 2024 12:12
@aglab2
Copy link
Collaborator Author

aglab2 commented May 30, 2024

Do not merge till PR Fast-64/fast64#356 is in main

@Lilaa3
Copy link
Contributor

Lilaa3 commented May 31, 2024

someone should really merge that honestly

Copy link
Collaborator

@someone2639 someone2639 left a comment

Choose a reason for hiding this comment

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

upstream fix seems to have been merged in so there should be no blockers here

@gheskett
Copy link
Collaborator

gheskett commented Sep 5, 2024

Is there anything else here to be added to fast64, especially now that HackerSM64 detection is a thing that actually exists?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request high priority Important, non-critical issue or feature / high priority monkaS monkaS needs verification Needs to be tested to ensure functionality
Projects
Status: In progress
Development

Successfully merging this pull request may close these issues.

6 participants