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

Menu work #452

Merged
merged 3 commits into from
Jan 22, 2018
Merged

Menu work #452

merged 3 commits into from
Jan 22, 2018

Conversation

mid-kid
Copy link
Member

@mid-kid mid-kid commented Dec 30, 2017

This PR will contain multiple commits that properly document and abstract menu data and functions.
Putting this here so people can look at it and provide feedback for the time being.

wram.asm Outdated
; MenuData2

wMenuData2Flags:: ; cf91
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be split into wMenuData2Flags, wMenuData2_2DMenuFlags, and wMenuData2_ScrollingMenuFlags in those unions? Then the comments about different bit meanings would also go in their respective sections, engine/scrolling_menu.asm can use wMenuData2_ScrollingMenuFlags, etc.

Copy link
Member Author

Choose a reason for hiding this comment

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

No, because some flags and code checking those flags are used for several different menus. See, for example, GetMenuTextStartCoord, which checks bit 6 and 7 of the flags, and is used by both PlaceVerticalMenuItems and Place2DMenuItemStrings.

@roukaour
Copy link
Contributor

roukaour commented Jan 13, 2018

Regarding macros for menu data, I think dealing with (x, y, width, height) would be easier than (y1, x1, y2, x2), particularly in map scripts.

A sample menu from CeladonDeptStore6F.asm:

.MenuData:
	db $40 ; flags
	db 02, 00 ; start coords
	db 11, 19 ; end coords
	dw .MenuData2
	db 1 ; default option

.MenuData2:
	db $80 ; flags
	db 4 ; items
	db "FRESH WATER  ¥200@"
	db "SODA POP     ¥300@"
	db "LEMONADE     ¥350@"
	db "CANCEL@"

And with a menu_dims macro:

.MenuData:
	db VERTICALMENU_BIT_6 ; flags
	menu_dims 0, 2, 20, 10 ; x, y, width, height
	dw .MenuData2
	db 1 ; default option

.MenuData2:
	db MENUDATA2_BIT_7 ; flags
	db 4 ; items
	db "FRESH WATER  ¥200@"
	db "SODA POP     ¥300@"
	db "LEMONADE     ¥350@"
	db "CANCEL@"

Theoretically a single menudata macro could pack in all the parameters:

.MenuData:
	menudata VERTICALMENU_BIT_6, 0, 2, 20, 10, .MenuData2, 1

But that could get long when combining multiple flags, and there's less room to add side comments for each bit of data. It also doesn't match the .MenuData2, which can't really accomodate a macro. (At most there could be menudata2 MENUDATA2_BIT_7, 4, but that goes against all the other db N item counts in map script files.)

Anyway, if menu_dims is acceptable, it can be added to map scripts without waiting for full menu flag documentation. Thoughts? (Edit: menu_size may be a better name.)

@aaaaaa123456789
Copy link
Contributor

On everything listed:

  1. As long as we don't have newline escapes, menudata is unreadable.
  2. VERTICALMENU_BIT_6 isn't more informative than $40. Only use constants if they are meaningful.
  3. menu_dims seems a lot more practical than the current method.
  4. No macro for the second menu structure is needed (and MenuData2 is a horrible name anyway).

@roukaour
Copy link
Contributor

  1. I agree menudata is unreadable.
  2. Right, that was just to illustrate "we'll have real constants here eventually".
  3. I'm glad menu_dims is looking good.
  4. Agreed.

@xCrystal
Copy link
Member

Could be wrong, but I think the end coords gives more useful information at first glance than the size, because it lets you picture the four corners of the box right away. That said, a menu_coords macro is definitely more telling than one or two random db.

Anyway, I guess it's up to preference so I don't really mind how it turns out.

Gave them better names, moved the flags out of the union, and documented
the w2DMenuFlags1.
Added a bunch of constants for the bitflags of each kind of menu. Some
are slightly vague or too long, but I'm not sure how to else name them.
While slightly shitty, it's as complete as I'm able to get it. Any
improvements are welcome.
@mid-kid mid-kid changed the title [Do not merge] Menu work Menu work Jan 14, 2018
@mid-kid
Copy link
Member Author

mid-kid commented Jan 14, 2018

Ready for merging.

roukaour added a commit to Rangi42/pokecrystal that referenced this pull request Jan 15, 2018
…lve PR pret#452)

# Conflicts:
#	data/player_names.asm
#	engine/battle/menu.asm
#	engine/billspc.asm
#	engine/billspctop.asm
#	engine/buy_sell_toss.asm
#	engine/decorations.asm
#	engine/delete_save_change_clock.asm
#	engine/events/buena.asm
#	engine/events/elevator.asm
#	engine/events/kurt.asm
#	engine/events/mom.asm
#	engine/events/move_tutor.asm
#	engine/events/pokecenter_pc.asm
#	engine/events/pokepic.asm
#	engine/events/std_scripts.asm
#	engine/events/unown_walls.asm
#	engine/init_gender.asm
#	engine/intro_menu.asm
#	engine/mail.asm
#	engine/main_menu.asm
#	engine/mart.asm
#	engine/menu_2.asm
#	engine/mon_menu.asm
#	engine/pack.asm
#	engine/slot_machine.asm
#	engine/start_menu.asm
#	home/menu.asm
#	maps/CeladonDeptStore6F.asm
#	maps/CeladonGameCornerPrizeRoom.asm
#	maps/DragonShrine.asm
#	maps/EarlsPokemonAcademy.asm
#	maps/GoldenrodCity.asm
#	maps/GoldenrodDeptStore6F.asm
#	maps/GoldenrodGameCorner.asm
#	maps/GoldenrodPokeComCenter2FMobile.asm
#	mobile/mobile_12.asm
#	mobile/mobile_12_2.asm
#	mobile/mobile_22.asm
#	mobile/mobile_22_2.asm
#	mobile/mobile_40.asm
#	mobile/mobile_45.asm
#	mobile/mobile_45_sprite_engine.asm
#	mobile/mobile_46.asm
#	mobile/mobile_5c.asm
#	mobile/mobile_5f.asm
#	mobile/mobile_menu.asm
@yenatch yenatch merged commit d38ed29 into pret:master Jan 22, 2018
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.

5 participants