* Pass boolean for `freezeFlag` to `Actor_SetDropFlag`
* Use `else if`s in `func_80835F44`
* Cleanup `transitionCtx` usage of instance data
* `TransitionContext.setEnvColor` -> `TransitionContext.setUnkColor`
* Fixup one comment
* linebreaks (#1)
* Run formatter
Co-authored-by: fig02 <fig02srl@gmail.com>
* Name rsp ucodes and surrounding cleanup
* Fix comments alignment and remove unnecessary & in sys_ucode
* Change capitalization of sys_ucode variables
* Remove unnecessary externs for aspMain
* Improvements to Jpeg_ScheduleDecoderTask
* Split unknown rsp code into a separate file, spacing
* Format all handwritten asm and document some
* Use c preprocessor for constants
* Fix
* Fix PI_STATUS_ERROR, some label improvements
* Avoid hi/lo for constants
* Some more comments
* Properly mark functions as functions and their sizes
* Fix merge
* Improvements
* Review suggestions, rework procedure start/end macros to be more like libreultra
* Move IPL3 symbol definitions into ipl3.s
* Fix undefined_syms, add include and language guards to asm.h and fix the comment in gbi.h
* Consistent hex capitalization, add some MIPS builtin defines to CC_CHECK to behave properly
* Add -no-pad-sections assembler option and clean up alignment in gu files and bzero
* Further suggestions and improvements
* Matrix conversion function clarifications
* Fix passing AVOID_UB to gcc
* Suggestions
* Suggestions, global interrupt mask improvements
* Further suggestions, interrupt mask comments
* Comments fixes, rdb.h
* Switch from # comments to // comments, remove unnecesary .set gp=64 directives
* Further review suggestions
* Missed one
* transition type enum
* mode enum, start documenting some types
* some more
* use enums for entrance table
* entrance table filled out
* sceneLoadFlag -> transitionTrigger
* sandstorm state/mode/type
* done i think
* fixes
* clean up circle weirdness
* circle use enum + fix texture names
* fix
* how did that even happen lol
* jesus
* review2
* some more review
* most review, still some more to do
* new transition trigger names
* some of review
* next type default
* message queues, threads, and surroundings cleanup
* Format, make the formatter prefer clang-format-11 if found
* Fix __osThreadTail type
* Q -> Queue, thread defines renamed
* Reformat, add missing NULL
* Suggested changes and further casting cleanup
* Reformat
* padmgr name fixes
* Fix typo `tranform` -> `transform`
* (may be controversial) `indexes` (noun) -> `indices`, both are correct but `indices` prevails in the repo currently, and seems prefered in technical contexts
* Fixup one `@bug`
* Fix typo `relfect` -> `reflect`
* Rename and `@bug` `DemoTreLgt_OverrideLimbDraw`
* Fixup `PLAYER_AP_` to `ARROW_` type conversion
* `posModel` -> `modelPos`
* `effSpawnPosModel` -> `effSpawnModelPos`
* Gohma decay bug: Verified that timers do not prevent UB access
* Reorder summation in a more meaningful way
Co-authored-by: JoshDuMan <40190173+JoshDuMan@users.noreply.github.com>
* Introduce and use more enums and defines for equips and data involved in player drawing
* `EQUIP_TYPE_`, `EQUIP_INV_`, `EQUIP_VALUE_`
* Improve doc on ItemEquips/Inventory's `equipment` field
* Run formatter
* Add `PLAYER_MODELGROUPENTRY_*` for second index of `gPlayerModelTypes`
* Name `PLAYER_MODELGROUPENTRY_*` from existing documentation
* Partial doc on `PLAYER_MODELTYPE_*`
* Some doc on `PLAYER_MODELGROUP_*`
* Run formatter
* Name and some doc/cleanup on the various player draw functions
* Name symbols used for player position in pause menu
* Name player override/post limb draw callbacks
* `BOOTS_NORMAL` -> `BOOTS_KOKIRI`
* `EQUIP_INV_SWORD_GIANTKNIFE` -> `EQUIP_INV_SWORD_BROKENGIANTKNIFE`
* `currentSword` -> `currentSwordItem` to avoid confusion with `PlayerSword` enum
* Make one constant more explicit
* Document severe bug (not an issue by luck)
* Some doc on `PLAYER_MODELTYPE_RH_FF`
* Actually name most of `PlayerModelType`
* Actually name most of `PlayerModelGroup` and improve comments
* Make equipment in debug save data use equip enums
* Name symbol for computing player's `bodyPartsPos` and fix struct access
* Name right/left `HandType` symbols
* Add `PlayerBodyPart` enum
* Run formatter
* Introduce defines for pause player render dimensions and cleanup viewport usage in pause drawing
* Document `playerSegment` related usage
* Run formatter
* Revert player bodypart changes
* Revert equips doc changes
* Revert player model and anim data changes
* Revert drawing-unrelated changes
* Move `sTunicColors` tunic comments to after data
* Fixup `Math3D_LineClosestToPoint`
* `gDodongosCavernBossLavaFloorTex` is 32x64
* Name empty-dlist-making functions `_EmptyDList`
* Fix typos
* transitionRate -> morphFrames
* Compare `xyzDistToPlayerSq` to squared values
* Fix hookshot target/post collision header names being swapped
* Fix description of `z_bg_mizu_movebg.c`
* Add scene command comment to `func_80098508` to match other scene command handlers
* Some fixup in `Camera_Demo5`
* `1` -> `ALLOCTYPE_ABSOLUTE` in comment on `ActorContext.absoluteSpace`
* Force dword alignment on OSPifRam
* force_structure_alignment on each Font buffer
* Add ALIGNED8 macro to gSaveContext to ensure alignment
* create alignment.h header and use ALIGN8 in PLAYER_LIMB_BUF_COUNT
* add comment in common_data
* Roman's suggestion
* player_limb_buf_count
Co-authored-by: fig02 <fig02srl@gmail.com>
* Properly align program stacks
* Enforce size being a multiple of 8 bytes
* Correct alignment calculation
* Use an ALIGN8 macro in the stack declaration macro
* merge master
* A little bit
* start docs on lens
* Delete Jenkinsfile
* Make sense of the numbers in `Actor_DrawLensOfTruthMask` f3dzex commands
* More comments on `Actor_DrawLens` f3dzex usage
* remove padding
* renames
* Simplify gfx comments
* Add some line breaks
* Remove undef
Co-authored-by: Dragorn421 <Dragorn421@users.noreply.github.com>
Co-authored-by: Louis <louist103@pop-os.localdomain>
* Cleanup parameters passed to `Actor_SpawnFloorDustRing`
* `10` -> `ARROW_NUT` for spawning EnArrow
* Missing empty line after declaration
* Introduce `ACTORCAT_MAX`
* Minor cleanup
* Run formatter
* `sgiDrawIds` -> `giDrawIds`
* `sDmaMgrDataExistError` -> `sDmaMgrIsRomCompressed` bool
* Add Makefile dependencies to rebuild some files when editing actor, object and effect tables in `include/tables/`
* Run formatter
* Revert "Add Makefile dependencies to rebuild some files when editing actor, object and effect tables in `include/tables/`"
This reverts commit c6b9a92139.
* some docs
* rename a bunch of callbacks
* more cleanup
* format
* last cleanup
* some updates
* more docs
* done i think
* name flag and format
* name unused variable
* comments
* revert unwanted change
* z64skin.h
* format
* Address review
* Rename PostLimbDraw callbacks to PostDraw and some PR addressing
* add true/false for setTranslation
* ahhhh
* remove "you"
Co-authored-by: fig02 <fig02srl@gmail.com>
* Name generic anim info structs and cleanup usage
* Make `EnDaiku` use generic struct
* Add `/* decimalid */` comments to all added enums
* small fixes
* Two more generic animinfo structs
* Cleanup usage of newly generic structs
* Cleanup x2 usage of newly generic structs
* `transitionRate` -> `morphFrames`
* Properly name generic anim structs (attempt to at least)
* `anim` -> `animation`
* fixes
* run formatter
* Use consistent prototypes for each actor's ChangeAnim helper
* run formatter
* minor cleanup
* Run formatter
* `Animation_ChangeInfo` -> `Animation_ChangeByInfo`
* Document `func_80033480` more
* More doc in `BgDdanKd`
* WIP Document BgDodoago
The dodongo skull light-eyes-with-bomb puzzle
Some TODOs, jank/legacy code which I'm unsure if it achieves something significant
* Finish documenting BgDodoago
* `BgDodoago_WaitExplosives_` -> `BgDodoago_WaitExplosives`
* Run formatter
* `dlistBuffer` -> `displayListHead`
* Cleanup `UNK_TYPE`, `UNK_PTR` usage
* Add some missing empty lines after declarations
* Remove some legacy comments from non-matching times
* Fix some grammar (mostly "it's"/"its")
* Use proper names for two symbols after ZAPD bugfix
* Cleanup `place_title_cards.xml`
* Use `NULL` to check against `D_8012D260` pointer
* Parentheses around some macro arguments
* wip proofread headers up to z64animation.h
* Fix some disp usage in EffectSsGMagma2
* Fix some incorrect SAC flags
* Remove some instances where bool was used
* Fix a few line numbers to be decimal
* Remove MatrixInternal and use Mtx instead
* Fix gs flag names to be plural for consistency
* Fix some hex numbers to be uppercase
* Fix instances of 0X for hex numbers
* Initial progress on z_message_PAL, very messy
* Fix merge
* Some more progress
* Fix merge
* More z_message_PAL
* Small progress
* More small progress
* message_data_static files OK
* Prepare z_message_tables
* Matched another function, small updates
* Attempt to use asm-processor static-symbols branch
* Refactor text id declarations
* Begin large text codes parser function
* Fix merge
* Refactor done
* Build OK, add color and highscore names
* Remove encoded text headers and automatically encode during build
* Fix kanfont
* Various cleanups
* DISP macros
* Another match aside data
* Further progress
* Small improvements
* Deduplicate magic values for text control codes, small improvements
* Tiny progress
* Minor cleanups
* Clean up z_message_PAL comment
* Progress on large functions
* Further progress on large functions
* Changes to mkldscript to link .data in the .rodata section
* data OK
* Few improvements
* Use gDPLoadTextureBlock macros where appropriate
* rm z_message_tables, progress on large functions
* 2 more matches
* Improvements
* Small progress
* More progress on big function
* progress
* match func_80107980
* match Message_Update
* match func_8010BED8
* done
* Progress on remaining large functions
* Small progress on largest function
* Another match, extract text and move to assets, improve text build system
* Small nonmatchings improvements
* docs wip
* Largest function maybe equivalent
* Fix merge
* Document do_action values, largest function is almost instruction-matching
* Rename NAVI do_action to NONE, as that appears to be how that value is used in practice
* Fix merge
* one match
* Last function is instruction-matching
* Fix
* Improvements thanks to engineer124
* Stack matched thanks to petrie911, now just a/v/low t regalloc issues, some cleanup
* More variables labeled, use text state enum everywhere
* More labels and names
* Fix
* Actor_IsTalking -> Actor_TalkRequested
* Match func_8010C39C and remove unused asm
* More docs
* Mostly ocarina related docs
* All msgModes named
* Fix assetclean
* Cleanup
* Extraction fixes and headers
* Suggestions
* Review suggestions
* Change text extraction again, only extract if the headers do not already exist
* Fix
* Use ast for charmap, fix assetclean for real this time
* Review suggestions
* BGM ids and ran formatter
* Review comments
* rename include_readonly to include_data_with_rodata
* Remove leading 0s in number directives
* Review suggestions for message_data_static
* textbox pos enum comments, rename several enum names from Message to TextBox
Co-authored-by: Thar0 <maximilianc64@gmail.com>
Co-authored-by: Zelllll <56516451+Zelllll@users.noreply.github.com>
Co-authored-by: petrie911 <pmontag@DESKTOP-LG8A167.localdomain>
Co-authored-by: Roman971 <romanlasnier@hotmail.com>
* matching split
* migrate progress
* split done and rodata migrated
* all data migrated to c
* start init
* progress
* progress
* progress
* progress
* progress
* progress
* copy done
* progress
* erase and settings done
* progress
* progress
* progress
* start keyboard
* progress
* progress
* progress
* progress
* Minor progress
* fix z_magic_dark issue
* func_80806F34 decomp'd
* verified equivalence
* one fix
* format
* merge petries work
* reorganizing
* lots of reorganizing and wraning fixing
* rename file
* remove language enum
* unwanted changes
* some symbol replacement, organization, and some names
* all symbols replaced, some organization
* some more cleanup
* continue docs
* Match the remaining functions in file_choose
* merge master
* select mode documented, all functions in file_choose.c named
* nameset functions named, some other cleaning
* some more general cleanup
* stub comments for cm, name a few things
* fix data and sizeof
* copy/erase functions and modes named
* rename assets, format
* change some struct members
* fixes
* review1
* fix maching error
* extract VTX data
* re add werror to ZAPD
* review 2
* fix
* remove file boundary padding
* remove zeroes
* review3
* change skybox stuff
* changes to vs etc
Co-authored-by: KrimtonZ <krimtonz@gmail.com>
Co-authored-by: Thar0 <17233964+Thar0@users.noreply.github.com>
Co-authored-by: mzxrules <mzxrules@gmail.com>
Co-authored-by: petrie911 <pmontag@DESKTOP-LG8A167.localdomain>
Co-authored-by: Roman971 <romanlasnier@hotmail.com>
Co-authored-by: Louis <louist103@gmail.com>
* remove fake match
* u32 segment -> void* and add symbols to sEntranceCutsceneTable
* rename some symbols
* remove if1
* add the last missing symbol
* add symbols for almost all extrence textures
* finish adding the symbols
* Remove comment
* spelling fix
* Fig's and Dragorn's fixes
* add symbols to unkknown array
* format
* romans fixes
* Some renames
* Some renames
* Some more renames
* pageDown
* Last name update
* Translate scene entries
* formatter
* Review
* A bunch of suggestions
* Run formatter
* Roman's suggestions
* Add parens around params usage in VEC_SET macro
* Remove unnecessary space character in a xml
* Use defines instead of magic values in head/tail magic comments
* Use `OS_USEC_TO_CYCLES` to make a time look better in `Graph_TaskSet00`
* `0x25800` -> `sizeof(u16[SCREEN_HEIGHT][SCREEN_WIDTH])`
* `0x803DA800` -> `0x80400000 - frame buffer size`
* Use `OS_VI_` defines instead of hex
* Add empty line after some variable declarations
* Remove unused `extern CutsceneData` in `z_bg_dy_yoseizo.c`
* `Matrix_MtxFToYXZRotS` does not use `MTXMODE_`
* Use `MTXMODE_` more
* Remove `ASCII_TO_U32`, use `'IS64'`
* Add explicit `!= NULL` in some ternaries
* Use `INV_CONTENT`, `AMMO` macros more
* Use `PLAYER_AP_` enum more to compare to `Player#heldItemActionParam`
* Get rid of lowercase hex (outside libultra)
* `gWindMill*` -> `gWindmill*`
* Format and small fix enums in `z_boss_mo.h`
* Use `CHECK_BTN_ANY` more
* Fix xz/xy mistake in comment in tektite
* Rephrase comments mentioning "the devs" in a more neutral way
* Clean-up some objectively useless parens
* Fix some negative values written as u16 instead of s16 in ichains
* `SKJ_ACTON_` -> `SKJ_ACTION_`
* Run formatter
* Fix unk_ offset of `TransformUpdateIndex#unk_10` -> `unk_0E`
* Remove comments using in-game text
* Remove `U` suffix from integer literals
* Revert "Remove `ASCII_TO_U32`, use `'IS64'`"
This reverts commit c801337dde.
* Use `PLAYER_STR_*` to compare to `CUR_UPG_VALUE(UPG_STRENGTH)`
* Add empty line after decl x2
* Revert "Use `PLAYER_STR_*` to compare to `CUR_UPG_VALUE(UPG_STRENGTH)`"
This reverts commit d80bdb32da.
* Make `CUR_UPG_VALUE(UPG_STRENGTH)` compare to integers (eventually needs its own enum)
* Only use `PLAYER_SHIELD_` enum with `Player#currentShield`
* Only use `PLAYER_TUNIC_` enum with `Player#currentTunic`
* Fix all headers to comply with C standard
* fix a file in libultra
* Update include/stdbool.h
Co-authored-by: Dragorn421 <Dragorn421@users.noreply.github.com>
* Update stdbool.h
* Update z64animation.h
Co-authored-by: Dragorn421 <Dragorn421@users.noreply.github.com>
* Match a few
* Remove cast
* BossGanon2_Destroy
* Some struct layout
* Missed one
* Work on BossGanon2_Update
* Oops missed another
* Work on update more
* Get it to compile
* Work a bit on update
* func_808FD27C
* Cleanup a bit
* Fix incompatible pointer type
* Work more on update
* Multiply
* Fix janky cast regalloc
* Remove some casts
* Move switch case to right spot
* Tiny regalloc
* Fix stack
* Try to fix jump table
* Migrate bss
* Make data match
* Fix minor formatting
* Another minor formatting
* Replace hex with symbols
* Do display lists
* Vertices
* jnt items init
* Split gfx into separate file
* func_808FCF40
* func_808FCF5C
* func_808FD080
* func_808FD4D4
* func_808FF898
* func_808FFA24
* func_808FFAC8
* func_808FFBBC
* func_808FFC84
* use unk_313 as a bool
* func_808FFCFC
* func_808FFDB0
* Add commented static
* func_808FFEBC
* func_808FFFE0
* func_809000A0
* Remove useless cast in func_809000A0
* Remove useless temp
* Fix wrong branch
* BossGanon2_Update
* Fix implicit function declarations
* Type D_8090EB2C to zelda pointer
* Reorder include because why not
* Deduplicate undefined syms
* Add more to struct
* Work on func_808FD5F4
* Major work on func_808FD5F4
* Get func_808FD5F4 to compile
* Make unk_398 unsigned
* Work on func_808FD5F4
* Progress
* func_808FD210
* BossGanon2_Draw
* Make it compile
* Fix compiler warnings
* Fix branches func_808FD5F4
* re-nonmatching func_808FD5F4
* stuff
* fix math functions
* clang format
* more progress
* Replace bit stuff with type
* Merge master
* Add possibly janky things and fix while loop
* Why does this help
* Chained assignment
* Remove dubious thing
* Reorder thing
* Add possibly fake notices
* Use true and false
* Make func_808FD5F4 equivalent
* Massage stack a little
* func_80906AB0
* Correct texture params
* Whitespace is good
* Use decimal line numbers
* func_809069F8
* Work on func_809060E8
* Make funny loop less funny
* Float stuff
* Remove useless cast
* Merge upstream
* Fix compiler error in nonmatchings
* func_809060E8
* Change tex alloc to be full integer
* Add unk_234
* Correct size of unk_2F4
* Make textures u64[]
* Type textures to u64[]
* func_80905DA8
* func_80905674
* Replace some pi
* BossGanon2_PostLimbDraw2
* func_8090523C
* func_80904FC8
* func_80904D88
* BossGanon2_PostLimbDraw
* BossGanon2_OverrideLimbDraw
* func_8090464C
* func_80903F38
* Match func_80900890
* Move func_80906538 prototype
* Work on func_80902348
* Match func_80902348
* Add fake warning to array thing
* Work on func_8090120C
* Remove useless var from func_8090120C
* Work more on func_8090120C
* Move case probably into place func_8090120C
* Work on func_8090120C until 4914
* Add break to switch
* Pass player to startPlayerCutscene
* Work more on func_8090120C
* Work on func_80902524
* Move function prototype
* Match func_80902524
* Clean up sfx ids in func_808FD5F4
* Correct SFX_FLAG in func_8090120C
* Match func_8090120C
* Match func_80904340
* Change tex to u8[]
* Cleanup func_808FD5F4 a bit
* Clean up another inequality func_808FD5F4
* Match func_80904108
* Cleanup for PR
* Cleanup for PR part 2
* Cleanup data a bit and start on func_80906538
* Basically match func_80906538
* Match func_80906538
* Rename tex to shadowTexture
* Work on func_809034E4 a bit
* replace 255 with 0xFF for texture
* Correct NON_MATCHING guard
* Make func_809034E4 equivalent and not ugly!
* Reuse i in func_809034E4
* Minor cleanup in func_809034E4
* Remove unused asm
* Static all the data
* Switch to using assets xml for overlay graphics :)
* Remove now unused undefined syms
* Rename pad to D_809071FC
* Some cleanup
* Cleanup and name more stuff
* Name BossGanon2_DrawShadowTexture
* Rename to sZelda
* Correct array in func_809034E4.s
* Start using proper symbols for assets
* Add morre undefined syms comments
* Cleanup a bit
* Cleanup a bit more
* Use THIS in limb functions
* Update comments
* Use define for actor id
* Remove unnecessary padding in struct
* Rename rand_float to randFloat
* Update src/overlays/actors/ovl_Boss_Ganon2/z_boss_ganon2.c
Co-authored-by: Dragorn421 <Dragorn421@users.noreply.github.com>
* Various minor suggestions
* Replace shadowTexture with void* wherever possible
* Update src/overlays/actors/ovl_Boss_Ganon2/z_boss_ganon2.c
Co-authored-by: Dragorn421 <Dragorn421@users.noreply.github.com>
* Do multiply
* Name some variables from debug prints
* Update src/overlays/actors/ovl_Boss_Ganon2/z_boss_ganon2.c
Co-authored-by: Dragorn421 <Dragorn421@users.noreply.github.com>
* Use ARRAY_COUNT(sParticles) everywhere
* Put data and bss in _data.c
* Remove unecessary @bug s
* Minor suggestions
* Why are there so many timers
Co-authored-by: Dragorn421 <Dragorn421@users.noreply.github.com>
* being code_800EC960
* wip
* wip
* more audio progress
* migrate data in code_800E11F0
* wip
* make ok
* remove asm
* wip
* move some variables outside of gAudioContext to the AudioContext structure due to the size used in func_800E3094
* more progress
* wip
* wip
* split code_800E11F0
* migrate rodata in code_800E11F0
* match functions that couldn't because of data issues
* move code_800E4FE0 asm files
* wip
* more wip
* fix global functions, and forward declarations
* wip
* wip
* wip
* ocarina wip
* match a couple functions
* some progress
* separate some bss
* match func_800EDA3C
* some matching
* more matches
* migrate audio rodata
* some matches
* more matchess
* start on synthesis
* work on synthesis
* fix function declaration
* Merge branch 'master' into audio
* match a few more functions
* wip
* wip
* more matching, rename Audio_SetBGM to Audio_QueueSeqCmd
* name several audio functions, and audiocontext members
* more naming, rename code_800E11F0 to audio_load, code_800DAAC0 to audio_synthesis
* audio wip
* match a few more functions.
* wip
* add missing NON_MATCHING directive
* wip
* some matching, data reogranization
* match cursed function
* wip
* wip
* formatting
* remove prefix from struct memebers
* missed function rename
* review
Co-authored-by: fig02 <fig02srl@gmail.com>
* z_vr_box.c progress
* Rename dpList to dlistBuf and SKYBOX_CLEAR_SKY to SKYBOX_NORMAL_SKY
* Fix
* renamed dlistBuf to dListBuf and split large string further
* Add symbols, does NOT build OK
* parameter OK with syms
* Use `static` more and inline arrays
* wip docs
* Start on Interface_Draw, NOT OK yet
* rename variables since interface dev is weird
* nonmatching for now
* kaleido
* Update src/code/z_parameter.c
Co-authored-by: louist103 <35883445+louist103@users.noreply.github.com>
* Make most texture rectangles use shifts
* fix symbols for all interface things
* done with interface_draw for now
* Update z_parameter.c
* format files
* fix wrong function calls
* fix bss
* improvements in matching
* remove button enum for now
* (void)0 to fix warning
* make conditional look nicer
* fix
* fix wrong combiner
* Update src/overlays/actors/ovl_kaleido_scope/z_lmap_mark.c
Co-authored-by: Roman971 <32455037+Roman971@users.noreply.github.com>
* format
* remove "HUD" naming
* review suggestions
Co-authored-by: louist103 <35883445+louist103@users.noreply.github.com>
Co-authored-by: Roman971 <32455037+Roman971@users.noreply.github.com>
* Improve z_jpeg documentation
* JpegWork::data is probably better than address
* Rename Jpeg_SendTask to Jpeg_ScheduleDecoderTask
* Rename njpegUCode to gJpegUCode
* Rename GetU16 to GetUnalignedU16 and improve docs
* ViMode OK
* Fix variable name in regconvert tool
* ViMode: fix formatting
* Add nonmatching impl for ViMode_Configure, respond to PR
- Fixed struct field naming (unkXX -> unk_XX)
- ViMode_Configure args are not flags for top/left etc; seem to be for
video mode
- Used @Random06457's implementation of `ViMode_LogPrint`
* Use plain LOG_ADDRESS in ViMode_LogPrint
* Incorporate PR feedback from Roman971
