Skip to content

Add "edit" and "open folder" actions for dropdowns#776

Merged
oldmud0 merged 25 commits into
masterfrom
more-dropdown-actions
Jul 31, 2022
Merged

Add "edit" and "open folder" actions for dropdowns#776
oldmud0 merged 25 commits into
masterfrom
more-dropdown-actions

Conversation

@Crystalwarrior
Copy link
Copy Markdown
Contributor

@Crystalwarrior Crystalwarrior commented Jun 2, 2022

  • CharSelect char button with edit char.ini, open this char folder, open base characters folder
  • pos dropdown with open this background, open base backgrounds folder
  • text color dropdown with open chat_config.ini
  • evidence button with open base evidence folder
  • sound list with open base sounds folder
  • music list with open base music folder

* CharSelect char button with edit char.ini, open this char folder, open characters folder
* pos dropdown with open this background, open background(s) folder
* text color dropdown with open chat_config.ini
* evidence button with open evidence folder
* sound list with open sounds folder
* music list with open music folder
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Comment thread src/charselect.cpp Outdated
Comment thread src/charselect.cpp Outdated
Comment thread src/courtroom.cpp Outdated
Comment thread src/courtroom.cpp Outdated
Comment thread src/courtroom.cpp Outdated
Comment thread src/courtroom.cpp
Comment thread src/courtroom.cpp
Comment thread src/courtroom.cpp
Comment thread src/courtroom.cpp Outdated
Comment thread src/courtroom.cpp
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Comment thread src/aooptionsdialog.cpp Outdated
stonedDiscord and others added 11 commits June 6, 2022 19:19
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Comment thread src/aooptionsdialog.cpp Outdated
Comment thread src/charselect.cpp Outdated
Comment thread src/charselect.cpp
Comment thread src/charselect.cpp
Comment thread src/courtroom.cpp Outdated
Comment thread src/courtroom.cpp
Comment thread src/courtroom.cpp Outdated
Comment thread src/courtroom.cpp Outdated
Comment thread src/courtroom.cpp
Comment thread src/courtroom.cpp Outdated
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Comment thread src/charselect.cpp
Comment thread src/charselect.cpp
Comment thread src/courtroom.cpp Outdated
Comment thread src/courtroom.cpp Outdated
Comment thread src/courtroom.cpp Outdated
Comment thread src/courtroom.cpp
Comment thread src/courtroom.cpp
Comment thread src/courtroom.cpp
Comment thread src/courtroom.cpp
Comment thread src/courtroom.cpp
@Crystalwarrior Crystalwarrior added this to the 2.10 milestone Jul 7, 2022
@TrickyLeifa TrickyLeifa added enhancement Request for functionality not present engine/assets Issues related to INI parsing, path resolution, and asset loading labels Jul 22, 2022
Copy link
Copy Markdown
Contributor

@TrickyLeifa TrickyLeifa left a comment

Choose a reason for hiding this comment

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

Barely any of this works and the only that does work needs more work.

Comment thread src/aooptionsdialog.cpp
Comment thread src/courtroom.cpp
Comment thread src/courtroom.cpp
Comment thread src/courtroom.cpp
@Crystalwarrior Crystalwarrior dismissed TrickyLeifa’s stale review July 27, 2022 17:20

Suggestions implemented

Copy link
Copy Markdown
Member

@oldmud0 oldmud0 left a comment

Choose a reason for hiding this comment

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

There's some good stuff and some really bad stuff

Comment thread src/aooptionsdialog.cpp Outdated
Comment thread src/charselect.cpp Outdated
Comment thread src/courtroom.cpp Outdated
Comment thread src/courtroom.cpp
Comment thread src/courtroom.cpp Outdated
Comment thread src/courtroom.cpp
Comment thread src/courtroom.cpp
Comment thread src/courtroom.cpp
Comment thread src/courtroom.cpp Outdated
@stonedDiscord
Copy link
Copy Markdown
Member

how is this important for 2.10

@stonedDiscord stonedDiscord removed this from the 2.10 milestone Jul 30, 2022
@Crystalwarrior Crystalwarrior requested a review from oldmud0 July 30, 2022 17:17
@Crystalwarrior Crystalwarrior requested review from stonedDiscord and removed request for TrickyLeifa July 31, 2022 05:06
@Crystalwarrior
Copy link
Copy Markdown
Contributor Author

this PR's done, implemented Longbyte's and Leifa's requests

Comment thread src/charselect.cpp Outdated
Comment on lines +225 to +230
menu->addAction(QString("Open base characters folder"), this,
[=] {
QString p_path = ao_app->get_real_path(VPath("characters/"));
QString p_path = ao_app->get_base_path() + "characters/";
if (!dir_exists(p_path)) {
return;
}
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

what I really mean to say is, this button is not really useful. The only reason you want to open this folder is if you are looking for a character or you want to add a character, and in both cases the way to do it is with a different button.

Comment thread src/courtroom.cpp Outdated
menu->addSeparator();
menu->addAction(QString("Play"), this, &Courtroom::on_sfx_play_clicked);
// SFX is not "Nothing" or "Default"?
if (get_char_sfx() != "0" && get_char_sfx() != "1") {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think - is also common for sfx.

- Base characters and base backgrounds folder buttons aren't generally
  u		seful as usually these are mounted somewhere else. Easier to open
  the character or background folder and go up one level.
- 	I decided to give the evidence, music, and sound lists the benefit
  of the doubt and kept their buttons for opening the base path,
  since there aren't many places to put an "open file location"
  button.
- Added "-" to the list of SFX names to ignore when previewing.
Copy link
Copy Markdown
Member

@oldmud0 oldmud0 left a comment

Choose a reason for hiding this comment

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

approved with some modifications. I'm not really liking the "open base folder" concept as it does not go well with mounts. If there is a strong demand it may be worth making a file browser with QFileDialog and setting a proxy model so that it uses the vfs instead of the native file system.

@oldmud0 oldmud0 merged commit b9aa203 into master Jul 31, 2022
@oldmud0 oldmud0 deleted the more-dropdown-actions branch July 31, 2022 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

engine/assets Issues related to INI parsing, path resolution, and asset loading enhancement Request for functionality not present

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants