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

Removed object list limit for OOT #172

Merged
merged 1 commit into from
Nov 14, 2022

Conversation

kurethedead
Copy link
Collaborator

A warning message appears instead in the object list panel if going over the limit.

@Yanis42
Copy link
Contributor

Yanis42 commented Nov 11, 2022

might be worth to mention the location of the alloc size

also idk how this is working exactly but users might need to increase this if they want more objects in the list (this number might be 16 for the room's object list + Link's object + gameplay_keep + global scene object (dangeon_keep or field_keep))

maybe these informations could go in the oot readme on fast64?

@kurethedead
Copy link
Collaborator Author

Honestly I don't know much about the process myself - I think someone else who can explain it better should add it to the PR.

@sauraen
Copy link
Contributor

sauraen commented Nov 13, 2022

I think this should either just go in as is--it's a warning, if you have modified the decomp code and you know you're good then you can ignore the warning--or there should be some way to specify or auto-detect from the decomp the current list size. The error message is a little misleading as it's a maximum number of objects in the list, not a maximum memory size (well you can hit that too). But even as is it's better than just giving an error always.

@kurethedead
Copy link
Collaborator Author

I've asked @krm01 to add a small description to the readme - after that we can merge.

@fig02
Copy link

fig02 commented Nov 13, 2022

Is there any possibility for a checkbox you can set to disable the warning, or something along those lines? Or just making the checkbox toggle the ability to go over the limit?

Its obviously not the biggest deal to just ignore the warning if you know your stuff is set up properly, but I think itd be nice to disable it, so you dont see it everytime youre exporting a new scene. The warnings should remain for important issues, instead of having the user train themselves to ignore it imo

@BeverlyBean
Copy link

a button just for that warning sounds unnecessary

@Zelllll
Copy link
Contributor

Zelllll commented Nov 13, 2022

I agree that it isn't really necessary. If the user is aware that they have already changed the limit in their project, then they should just be able to ignore the warning since they know it doesn't apply to them.

@kurethedead
Copy link
Collaborator Author

This isn't a warning on export, just a notice at the top of the object list if there's more than 16 objects. It prints to the console on export, but the user isn't going to be seeing that.

@fig02
Copy link

fig02 commented Nov 13, 2022

alright. I still dont think "the user can just ignore it" is better than a setting the user can explicitly set themselves to allow the cap to be bypassed. If one trains themselves to ignore that warning, it may lead to them ignoring others.
But, if its not on export, then my concern matters less

@BeverlyBean
Copy link

is it really that hard to ignore a simple warning??

@fig02
Copy link

fig02 commented Nov 13, 2022

i dont think you understand my point, but im not gonna bother explaining it to someone whos being snarky lmao

@BeverlyBean
Copy link

i guess i dont, i just dont think the "training yourself" thing applies to everybody

@Zelllll
Copy link
Contributor

Zelllll commented Nov 13, 2022

Personally, I just feel that this is a feature that would pollute the UI and ultimately add more confusion than necessary.

Copy link
Contributor

@Zelllll Zelllll left a comment

Choose a reason for hiding this comment

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

Good changes 👍🏻

Copy link

@fig02 fig02 left a comment

Choose a reason for hiding this comment

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

unrelated to the conversation above

fast64_internal/oot/oot_level_writer.py Show resolved Hide resolved
@Dragorn421
Copy link
Contributor

Merging, knowing the object list max length logic could use more work

@Dragorn421 Dragorn421 merged commit 1a7a3eb into Fast-64:main Nov 14, 2022
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.

None yet

7 participants