Skip to content

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

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

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

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)

}

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