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

Effects follow offset, add ini option to disable offset for effect #663

Merged
merged 3 commits into from
Feb 21, 2022

Conversation

Salanto
Copy link
Contributor

@Salanto Salanto commented Feb 19, 2022

This is pretty hacky and ugly, but the offset data is necessary to move the effect layer in relation to the character.
closes #639

This is pretty hacky, but the offset data is necessary to move the effect layer in relation to the character.
@Salanto Salanto requested a review from oldmud0 February 19, 2022 02:48
Copy link
Contributor

@Crystalwarrior Crystalwarrior left a comment

Choose a reason for hiding this comment

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

Need to make ignore_offsets work lol

src/courtroom.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@Crystalwarrior Crystalwarrior 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

int self_offset = self_offsets[0].toInt();
int self_offset_v;
if (self_offsets.length() <= 1)
self_offset_v = 0;
else
self_offset_v = self_offsets[1].toInt();
ui_vp_player_char->move(ui_viewport->width() * self_offset / 100, ui_viewport->height() * self_offset_v / 100);

//If an effect is ignoring the users offset, we force it to the default position of the viewport.
if (ao_app->get_effect_property(play_effect[0], current_char, "ignore_offset") == "true") {
Copy link
Member

@oldmud0 oldmud0 Feb 21, 2022

Choose a reason for hiding this comment

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

Very good, thank you. Let's document this.

EDIT: Let's pretend I didn't see that global variable there

@@ -3321,7 +3321,7 @@ void Courtroom::start_chat_ticking()
// handle expanded desk mods
switch(m_chatmessage[DESK_MOD].toInt()) {
case 4:
set_self_offset(m_chatmessage[SELF_OFFSET]);
set_self_offset(m_chatmessage[SELF_OFFSET], QString("||"));
Copy link
Member

Choose a reason for hiding this comment

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

QString("||") is not necessary. You may either use "||" or QStringLiteral("||") (some also prefer QLatin1String for efficiency, but the performance benefit is negligible)

@@ -3762,15 +3762,28 @@ void Courtroom::set_scene(QString f_desk_mod, QString f_side)
}
}

void Courtroom::set_self_offset(QString p_list) {
void Courtroom::set_self_offset(QString p_list, QString p_effect) {
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure why this method takes in a QString when it should really take in a QStringList.

@oldmud0 oldmud0 merged commit e4779f0 into master Feb 21, 2022
@in1tiate in1tiate deleted the fix/add-effects-offset branch July 18, 2022 22:49
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.

Effect Offsets w/ Character
3 participants