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

Fix #18665, #18559: Add API for getting g2 icons by name #18675

Merged
merged 28 commits into from
Dec 12, 2022

Conversation

spacek531
Copy link
Contributor

@spacek531 spacek531 commented Nov 25, 2022

This PR attempts to solve the issues created by #18497. It creates an API to get G2 icons by string. It allows plugin creators to use strings to refer to icons by name instead of a hardcoded number. Plugins with old targetAPI will map to the new scheme and plugins with old targetAPI will map back to the old scheme when requesting the icon from the button (just in case).

@spacek531 spacek531 force-pushed the api/g2-sprites-by-name branch 3 times, most recently from 0bb0057 to 5b761df Compare November 25, 2022 03:00
@spacek531
Copy link
Contributor Author

Does anyone know what's going on with the linux build?

src/openrct2/sprites.h Outdated Show resolved Hide resolved
@ltsSmitty
Copy link
Contributor

Since the type in the d.ts isn't an Enum, can you alphabetize the list of icon names for legibility?

Copy link
Member

@Basssiiie Basssiiie left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thank you so much for putting your time into this, much appreciated! 🤩

How it looks ingame:
image

src/openrct2/sprites.h Outdated Show resolved Hide resolved
@Gymnasiast Gymnasiast self-requested a review November 26, 2022 11:26
@spacek531
Copy link
Contributor Author

I've added a reverse map when getting the image number since it's theoretically possible a plugin would want that.

@spacek531 spacek531 marked this pull request as ready for review November 26, 2022 16:37
@Gymnasiast
Copy link
Member

From a quick glance, the implementation looks sound. Would want to test it, and have @IntelOrca do his own review.

@IntelOrca
Copy link
Contributor

Is there a reason to allow the enum for anything other than getIcon, and just call getIcon when populating the window desc.

@spacek531
Copy link
Contributor Author

It seemed easier to be able to write a string directly to the widget description than require getIcon. @Basssiiie @ltsSmitty you are the primary users, what do you think?

@Basssiiie
Copy link
Member

Basssiiie commented Dec 6, 2022

I'd say the enum as it is currently implemented is more convenient. Why add additional steps if it can be done in one step? It keeps it easy. :)

spacek531 and others added 2 commits December 7, 2022 15:19
Co-authored-by: Michael Steenbeek <m.o.steenbeek@gmail.com>
Copy link
Member

@Gymnasiast Gymnasiast left a comment

Choose a reason for hiding this comment

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

Re-requested a review from @Basssiiie to be sure, but looks good to me now.

Copy link
Member

@Basssiiie Basssiiie left a comment

Choose a reason for hiding this comment

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

Accessing sprites by G2 name:
Six Flags 2022-12-10 00-09-08

Accessing by legacy id for old plugins:
Six Flags 2022-12-10 00-09-10

Accessing via getIcon() on newer plugins:
Six Flags 2022-12-10 00-09-22

Code for test plugin:
const icons = [
	[ "empty",  32248 ],
	[ "placeholder",  29363 ],
	[ "logo",  29357 ],
	[ "logo_text",  29358 ],
	[ "fast_forward",  29359 ],
	[ "game_speed_indicator",  29360 ],
	[ "game_speed_indicator_double",  29361 ],
	[ "map_gen_land",  29362 ],
	[ "zoom_in",  29364 ],
	[ "zoom_in_background",  29365 ],
	[ "zoom_out",  29366 ],
	[ "zoom_out_background",  29367 ],
	[ "map_gen_trees",  29368 ],
	[ "map_gen_noise",  29369 ],
	[ "large_scenery",  29370 ],
	[ "small_scenery",  29371 ],
	[ "paths",  29372 ],
	[ "rct1_close_off",  29373 ],
	[ "rct1_close_off_pressed",  29374 ],
	[ "rct1_close_on",  29375 ],
	[ "rct1_close_on_pressed",  29376 ],
	[ "rct1_test_off",  29377 ],
	[ "rct1_test_off_pressed",  29378 ],
	[ "rct1_test_on",  29379 ],
	[ "rct1_test_on_pressed",  29380 ],
	[ "rct1_open_off",  29381 ],
	[ "rct1_open_off_pressed",  29382 ],
	[ "rct1_open_on",  29383 ],
	[ "rct1_open_on_pressed",  29384 ],
	[ "title_restart",  29385 ],
	[ "title_stop",  29386 ],
	[ "title_play",  29387 ],
	[ "title_skip",  29388 ],
	[ "cheats",  29389 ],
	[ "news_messages",  29414 ],
	[ "server_password",  29415 ],
	[ "multiplayer",  29416 ],
	[ "sort",  29433 ],
	[ "copy",  29434 ],
	[ "paste",  29435 ],
	[ "mute",  29442 ],
	[ "mute_pressed",  29443 ],
	[ "unmute",  29444 ],
	[ "unmute_pressed",  29445 ],
	[ "search",  29461 ],
	[ "eyedropper",  29467 ],
	[ "chat",  29468 ],
	[ "map_north",  29469 ],
	[ "map_north_pressed",  29470 ],
	[ "map_west",  29471 ],
	[ "map_west_pressed",  29472 ],
	[ "map_south",  29473 ],
	[ "map_south_pressed",  29474 ],
	[ "map_east",  29475 ],
	[ "map_east_pressed",  29476 ],
	[ "multiplayer_toolbar",  29477 ],
	[ "multiplayer_toolbar_pressed",  29478 ],
	[ "multiplayer_sync",  29479 ],
	[ "multiplayer_desync",  29480 ],
	[ "simulate",  29481 ],
	[ "rct1_simulate_off",  29482 ],
	[ "rct1_simulate_off_pressed",  29483 ],
	[ "rct1_simulate_on",  29484 ],
	[ "rct1_simulate_on_pressed",  29485 ],
	[ "normal_selection_6x6",  29486 ],
	[ "mountain_tool_even",  29487 ],
	[ "mountain_tool_odd",  29488 ],
	[ "scenery_scatter_low",  29489 ],
	[ "scenery_scatter_medium",  29490 ],
	[ "scenery_scatter_high",  29491 ],
	[ "view",  29494 ],
	[ "path_railings",  29495 ],
	[ "legacy_paths",  29496 ],
	[ "path_surface",  29497 ],
	[ "ride_station",  29498 ],
	[ "terrain_edge",  29499 ],
	[ "hide_vegetation",  29500 ],
	[ "hide_scenery",  29501 ],
	[ "hide_vehicles",  29502 ],
	[ "hide_supports",  29503 ],
	[ "hide_partial",  29504 ],
	[ "hide_full",  29505 ],
	[ "link_chain",  29506 ],
	[ "sideways_tab",  29507 ],
	[ "sideways_tab_active",  29508 ],
	[ "arrow_up",  29509 ],
	[ "arrow_down",  29510 ],
	[ "reload",  29511 ],
].sort(function(a, b) { return a[0].localeCompare(b[0]) });

registerPlugin({
	name: 'Test G2',
	version: '1',
	authors: ['Basssiiie'],
	type: 'remote',
	licence: 'MIT',
	//targetApiVersion: 65,
	main() {
		ui.registerMenuItem("Test G2 icons", function()
		{
			ui.openWindow({
				classification: "g2-test", title: "G2 test",
				x: 20, y: 50, width: 1800, height: 900,
				tabs: [
					{
						image: "zoom_in",
						widgets: create("new g2 name", icons, function(icon) { return icon[0] })
					},
					{
						image: 29499, // terrain edge shovel
						widgets: create("legacy id", icons, function(icon) { return icon[1] })
					},
					{
						image: context.getIcon("chat"),
						widgets: create("getIcon()", icons, function(icon) { return context.getIcon(icon[0]) })
					}
				]
			});
		});
	},
});


function create(name, icons, getId)
{
	var widgets = [
		{ type: "label", text: name, x: 100, y: 30, width: 200, height: 20 },
	];
	var columns = 14
	var span = 130;

	for (var i = 0; i < icons.length; i++)
	{
		var icon = icons[i];
		var id = getId(icon);
		var x = 20 + (i % columns) * span;
		var y = 50 + Math.floor(i / columns) * span;
		widgets.push({
			type: "button", image: id, x: x, y: y, width: 32, height: 32
		});
		widgets.push({
			type: "label", text: (icon[0] + "\n" + icon[1]), x: x, y: y + 40, width: span, height: 32
		});
	}
	return widgets;
}

So besides the few small issues, LGTM! Thank you all for your time.

src/openrct2/scripting/G2Names.hpp Outdated Show resolved Hide resolved
src/openrct2/scripting/G2Names.hpp Outdated Show resolved Hide resolved
src/openrct2/scripting/G2Names.hpp Outdated Show resolved Hide resolved
distribution/openrct2.d.ts Outdated Show resolved Hide resolved
Copy link
Member

@Gymnasiast Gymnasiast left a comment

Choose a reason for hiding this comment

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

Please fix the icon names as I requested.

Also change G2Names.hpp to IconNames.hpp - icons from g1 should be added as well (in another PR, preferably).

@IntelOrca
Copy link
Contributor

IntelOrca commented Dec 11, 2022

Also change G2Names.hpp to IconNames.hpp - icons from g1 should be added as well (in another PR, preferably).

Use IconNames.h. .hpp is for header-only implementations (i.e. fully implemented classes or functions without a .cpp file).

@Gymnasiast
Copy link
Member

Also change G2Names.hpp to IconNames.hpp - icons from g1 should be added as well (in another PR, preferably).

Use IconNames.h. .hpp is for header-only implementations (i.e. fully implemented classes or functions without a .cpp file).

There are a few functions defined in that file, so doesn’t .hpp make more sense here?

@IntelOrca
Copy link
Contributor

I didn't realise it was in scripting. I originally made most of the scripting code .hpp to reduce code and definition duplicates. But that has since been changed and now scripting files have been changed into .h and .cpp pairs. So I don't know what the plan is now with new stuff.

@Gymnasiast Gymnasiast added this to the v0.4.3 milestone Dec 12, 2022
@Gymnasiast Gymnasiast enabled auto-merge (squash) December 12, 2022 22:40
@Gymnasiast Gymnasiast merged commit 4410023 into OpenRCT2:develop Dec 12, 2022
@spacek531 spacek531 deleted the api/g2-sprites-by-name branch December 12, 2022 23:05
Gymnasiast added a commit that referenced this pull request Dec 14, 2022
- Feature: [#17782] The Flying Coaster now has access to boosters and can draw outside loops.
- Feature: [#17997] The Log Flume can now draw steep pieces down (if vehicle allows it).
- Feature: [#18312, objects#220, OpenSFX#13] New sound effects for the Hybrid and Single Rail roller coasters.
- Feature: [#18675] [Plugin] Plugins can refer to g2 image icons by name.
- Feature: [objects#173] Add alpine coaster vehicle.
- Feature: [objects#221] Add two extra jungle walls.
- Feature: [objects#225] Add log cabin roofs.
- Feature: [OpenMusic#14, OpenMusic#15, OpenMusic#18] Added Galaxy, Acid and Dodgems ride music styles.
- Improved: [#18013, #18016, #18018, #18019, #18514, objects#224] Added colour presets to Spiral Slide, Dodgems, Boat Hire, Flying Saucers, and Car Ride.
- Improved: [#18024] Clearer error messages when loading incompatible .park files.
- Improved: [#18192] Tycoon Park has been added to the Extras tab.
- Improved: [#18214] Competition scenarios have received their own section.
- Improved: [#18250] Added modern style file and folder pickers on Windows.
- Improved: [#18332] Allow Inverted Roller Coaster to draw boosters.
- Improved: [#18350] Changed ride vehicle list to have less padding.
- Improved: [#18422] Allow adding images to music objects.
- Improved: [#18428] [Plugin] Add widget description interfaces to documentation.
- Improved: [#18487] Mini Helicopters track can now draw spinning tunnels.
- Improved: [#18591] Order RollerCoaster Tycoon 2 scenarios by difficulty.
- Improved: [#18607] A new tab for all UCES Scenarios, if it’s installed.
- Improved: [#18621] OpenGL performance.
- Change: [#17677] Open campaign window from finished campaign news.
- Change: [#17998] Show cursor when using inverted mouse dragging.
- Change: [#18230] Make the large flat to steep pieces available on the corkscrew roller coaster without cheats.
- Change: [#18381] Convert custom invisible paths to the built-in ones.
- Change: [OpenSFX#11, OpenMusic#19] First implementation of official replacement asset packs for sound effects & music.
- Fix: [#1491] Clearance of the Cash Machine is too low (original bug).
- Fix: [#1519] “See-through rides” doesn't affect all rides (original bug).
- Fix: [#6341] “Unlock vehicle limits” does not allow setting fewer vehicles than the vehicle type requires.
- Fix: [#14312] Research ride type message incorrect.
- Fix: [#14425] Ride ratings do not skip unallocated ride ids.
- Fix: [#15969] Guests heading for ride use vanilla behaviour
- Fix: [#17067] Random Staff Patrol Area clicks.
- Fix: [#17316] Sides of River Rapids’ corners overlay other parts of the track.
- Fix: [#17657] When switching from buying land rights to buying construction rights, grid disables and won't re-enable afterwards.
- Fix: [#17763] Missing validation on invalid characters in file name.
- Fix: [#17853] Invention name tears while being dragged.
- Fix: [#18064] Unable to dismiss notification messages.
- Fix: [#18070] Underground entrance/exit shows through terrain walls (original bug).
- Fix: [#18094] Underground shops & facilities don't show when adjacent to non-underground path (original bug).
- Fix: [#18122] Ghosts count towards “Great scenery!” guest thought.
- Fix: [#18134] Underground on-ride photo section partially clips through adjacent terrain edge.
- Fix: [#18244] Invention DragWindow's starting position is inconsistent.
- Fix: [#18245] Guests stopping dead in their tracks at railway crossings.
- Fix: [#18257] Guests ‘waiting’ on extended railway crossings.
- Fix: [#18354] Overwrite alert does not show when save name has different casing on Windows.
- Fix: [#18379] Tunnel entrances for underground Mini Golf Hole E are not rendered correctly.
- Fix: [#18442] About window background is clickable.
- Fix: [#18449] [Plugin] Change type of listview widgets from 'scroll_view' to 'listview'.
- Fix: [#18453] Slow walking guests don't get across level crossings in time.
- Fix: [#18469] Land rights window buttons incorrectly disabled and markers remain visible indefinitely.
- Fix: [#18459] ‘Highlight path issues’ hides fences for paths with additions.
- Fix: [#18552] Trains clipping through helixes.
- Fix: [#18576] Cannot open parks with certain types of corrupt tile elements.
- Fix: [#18606] JSON objects do not take priority over the DAT files they supersede.
- Fix: [#18620] [Plugin] Crash when reading widget properties from windows that have both static and tab widgets.
- Fix: [#18653] Negative ratings multipliers do not appear in Vehicle tab.
- Fix: [#18696] Construction rights cannot be viewed after all are purchased.
- Fix: [#18720] Upwards helix is enabled for the Alpine Coaster, even when cheats are off.
- Fix: [#18755] Ferris Wheel and Circus ghosts not coloured correctly.
- Fix: [#18802] Game could crash when determining if a mechanic is heading to fix the ride blocking the path.
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.

Create API to fetch an icon G2 reoder PR #18497 breaks plugins using G2 sprites
6 participants