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

Upgrade to rgbds 0.4.0, and do miscellaneous fixes #705

Merged
merged 6 commits into from
Apr 6, 2020

Conversation

Rangi42
Copy link
Member

@Rangi42 Rangi42 commented Apr 4, 2020

  • fail for rgbds < 0.4.0
  • rst <Label>
  • ds <count>, <byte>
  • shift <N>
  • ASCII "\r"
  • Sorted .sym file

- fail for rgbds < 0.4.0
- `rst <Label>`
- `ds <count>, <byte>`
- `shift <N>`
- ASCII "\r"
- Sorted .sym file
- `hSecondsBackup` -> `hUnusedBackup`
- `ld bc, hBGMapAddress + 1` -> `bccoord -1, -2, 0`
- Identify some `.asm_XXX` labels
@Rangi42 Rangi42 changed the title [In progress] Upgrade to rgbds 0.4.0 Upgrade to rgbds 0.4.0 Apr 4, 2020
@Rangi42 Rangi42 changed the title Upgrade to rgbds 0.4.0 Upgrade to rgbds 0.4.0, and do miscellaneous fixes Apr 4, 2020
@Rangi42 Rangi42 requested a review from mid-kid April 4, 2020 20:04
@Rangi42
Copy link
Member Author

Rangi42 commented Apr 4, 2020

This does not yet resolve the DAYCARE/DAY_CARE inconsistency from #700 because I'm not sure which to go with. DAYCARE is more common, but in-game it's spelled "DAY-CARE".

This does not yet use LOAD/ENDL from rgbds 0.4.0 for WriteOAMDMACodeToHRAM.PushOAM and hTransferVirtualOAM because that fails to build the jr correctly.

@Rangi42 Rangi42 force-pushed the master branch 2 times, most recently from c4bb41d to e227f11 Compare April 4, 2020 21:47
@Rangi42
Copy link
Member Author

Rangi42 commented Apr 6, 2020

Re: #700, I think the DAY_CARE/DAYCARE constants are already fine. Most use "DAY_CARE" to match in-game "DAY-CARE": there's ENGINE_DAY_CARE_*, EVENT_DAY_CARE_*, the DAY_CARE map, SPRITE_DAY_CARE_MON_1/2, ROUTE34_DAY_CARE_MON_1/2, etc.

There are three kinds of constant that use "DAYCARE": DAYCAREMAN/DAYCARELADY_*_F, DAYCARETEXT_*, and the object_event constants DAYCARE_GRAMPS/GRANNY. All of those are following the pattern where spaces get taken out of names for brevity. For example, NEWBARKTOWN_SILVER, BATTLETOWERTEXT_INTRO, or LUCKYNUMBERSHOW_GAME_OVER_F.

Also this doesn't need to wait for LOAD/ENDL to work properly, since WriteOAMDMACodeToHRAM.PushOAM is fine without them (the hard-coded hTransferVirtualOAM:: ds 10 size is alright for now). So this should be ready to merge.

@mid-kid
Copy link
Member

mid-kid commented Apr 6, 2020

Except for the *TEXT constants, I think object event constants and event flags constants should match their respective map's names.

@Rangi42
Copy link
Member Author

Rangi42 commented Apr 6, 2020

EVENT_DAY_CARE_* do match map_const DAY_CARE. Do you mean that all the object_const_defs should be updated? Such as AZALEAMART_COOLTRAINER_MAZALEA_MART_COOLTRAINER_M?

@mid-kid
Copy link
Member

mid-kid commented Apr 6, 2020

In all honesty - yes. It's kind of arbitrary to make a difference with something that's related to a very exact map. But if it's a lot, it can be left for a different PR.

@Rangi42
Copy link
Member Author

Rangi42 commented Apr 6, 2020

If we get file-local defines (gbdev/rgbds#342), I'd like them to be even shorter: no need for the map name in every constant, with or without underscores, since they're only ever used locally in the map script.

e.g. LOCAL OBJ_COOLTRAINER_M

@mid-kid
Copy link
Member

mid-kid commented Apr 6, 2020

Agreed, it's something I wanted to consider as a result of #631, not only with defines, but also with the labels. The only two that need to be prefixed are the two that are exported, _MapScripts and _MapEvents.

FAQ.md Show resolved Hide resolved
INSTALL.md Outdated Show resolved Hide resolved
constants.asm Outdated Show resolved Hide resolved
@Rangi42
Copy link
Member Author

Rangi42 commented Apr 6, 2020

Shall I change this to just -Weverything and merge?

@mid-kid
Copy link
Member

mid-kid commented Apr 6, 2020

Sure, we can change it at any point. Be sure to include it in RGBASMFLAGS though.

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

2 participants