[FE8] Modular Minimug Box 2018: It's here!

Resources Assets
101

Glad you got things working!

I’ve got a few suggestions/notes/etc.:

Suggestions and notes

In MMBCurrentMPGetter.s, you’re still drawing the HP / tiles.

In the same file, I’d avoid the pattern push {lr}; bl ...; pop ...; and just push/pop lr at the start and end.

I see you’ve got the following definitions in CommonDefinitions.inc:

@ BWL values (JESTER)

  .set UnitCurrentMP, 0xD @14
  .set UnitMaxMP,     0xE @15

but every MP-getting operation does something like this (I’ve included my own comments, prefixed with @@ instead of the usual @.)

...
@@ Pointer to unit's battle buffer in r0

@@ The MMB provides `UnitCharacterData` in `CommonDefinitions.inc`, so:
@@ ldr r0, [r0, #UnitCharacterData]
ldr   r0, [r0, #0] @ Load class data of unit

@@ You'd have to add definitions for the character struct yourself, but this
@@ should probably end up as something like
@@ ldrb r0, [r0, #CharacterDataIndex]
ldrb  r0, [r0, #4] @ Load character index
ldr   r1, =BWL_GetEntry
mov   lr, r1
bllr

@@ `BWL_GetEntry` returns a pointer. I'd make a `NULL` definition in
@@ `CommonDefinitions.inc` and use that here.
cmp   r0, #0
beq   NoBWLData

  @@ This offset, according to `MMBCurrentMPGetter.s` is the current MP, but
  @@ `CommonDefinitions.inc` lists the offset as 0xD. The max MP is similarly shifted.
  @@ Definitions are used precisely to prevent these kinds of mismatches.
  @@ You also begin these snippets with comments that refer to the offsets and not
  @@ the definitions, which creates another opportunity to be wrong.
  add   r0, #0xC
  ldrb  r0, [r0]
  b     CheckLimit

NoBWLData:

  mov   r0, #0

CheckLimit:
...

Semi-related: during MMBDrawMP.s (but not the current/map getters) you copy/pasted capping functionality from your edits to the HP display, which all refer to HP rather than MP.

A really subjective suggestion: the MMB supports drawing the Skill System’s charge stat as it was originally implemented. Since not every unit would have charge skills, the MMB would display -- for them (by passing 0xFF to the number drawing function). I think that might look cleaner than drawing zeroes for the MP display.

102

Hey Zane, thanks for getting back to me.

In MMBCurrentMPGetter.s, you’re still drawing the HP / tiles.

Ah, old code. Removed now.

In the same file, I’d avoid the pattern push {lr}; bl ...; pop ...; and just push/pop lr at the start and end.

I wasn’t sure if it was needed, but now that I’ve tested apparently it isn’t. Removed :+1:

I see you’ve got the following definitions in CommonDefinitions.inc:

@ BWL values (JESTER)

  .set UnitCurrentMP, 0xD @14
  .set UnitMaxMP,     0xE @15

but every MP-getting operation does something like this (I’ve included my own comments, prefixed with @@ instead of the usual @.)

I’ve cleaned up the common definitions to align with the correct BWL offsets and actually use those definitions now.

Semi-related: during MMBDrawMP.s (but not the current/map getters) you copy/pasted capping functionality from your edits to the HP display, which all refer to HP rather than MP.

This is fine. The value in r0 at that time is the BWL MP value that’s been returned. I intend to use the same capping functionality.

A really subjective suggestion: the MMB supports drawing the Skill System’s charge stat as it was originally implemented. Since not every unit would have charge skills, the MMB would display -- for them (by passing 0xFF to the number drawing function). I think that might look cleaner than drawing zeroes for the MP display.

Is this what @Struedelmuffin used for Reforged? (If you’ve played it).

Neat if so, I might reuse that for something else.

Thanks for the great feedback :slight_smile: