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

Goal_action #2198

Draft
wants to merge 13 commits into
base: master
Choose a base branch
from
Draft

Conversation

SkaldetSkaeg
Copy link

@SkaldetSkaeg SkaldetSkaeg commented Nov 2, 2024

Описание PR

Мемов ради сделал специальную кнопку на медведя, которая выкрикивает "Гол!"

Перенес всё в ActionGrant. Сделал уникальный ивент позволяющий конструировать кнопки с различными звуками и текстом в чат.

Медиа
Видео с раннего билда, сейчас есть и делэй на действии и фразы в чате
https://github.com/user-attachments/assets/de3ad361-6064-4412-878a-689d266192f4

Проверки

  • PR полностью завершён и мне не нужна помощь чтобы его закончить.
  • Я ознакомился с наставлениями по работе с репозиторием и следовал им при создании PR'а.
  • Я внимательно просмотрел все свои изменения и багов в них не нашёл.
  • Я запускал локальный сервер со своими изменениями и всё протестировал.
  • Я добавил скриншот/видео демонстрации PR в игре, или этот PR этого не требует.

Изменения
🆑

  • add: Медведям добавлена специальная звуковая кнопка:)

@github-actions github-actions bot added Changes: Localization Изменение затронуло файлы ".ftl" Changes: Prototypes Изменение затронуло файлы ".yml" кроме неймспейса "maps" Changes: C# Изменение затронуло файлы ".cs" labels Nov 2, 2024
@SkaldetSkaeg SkaldetSkaeg changed the title Gol_action Goal_action Nov 2, 2024
@stalengd stalengd requested a review from Ady4ik November 2, 2024 14:51
@SkaldetSkaeg
Copy link
Author

@Ady4ik сделай ревью, выскажи, чтобы чисто по коду только сталось

@Kirus59 Kirus59 added the Need to be discussed Требуется обсуждение перед аппрувом/мёрджем label Nov 4, 2024
Copy link

@Ady4ik Ady4ik left a comment

Choose a reason for hiding this comment

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

Медведь орет гол - что может быть лучше? Только КД на экшн сделать и в целом вопросов у меня нет)

Comment on lines +21 to +24
/// <summary>
/// Just handler of a ShoutActionEvent.
/// If there is no sound or phrase it won't do anything.
/// </summary>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Я так думаю это должно быть документацией к классу, так что должно быть прямо перед ним

namespace Content.Shared.SS220.Shout;
/// <summary>
/// Event for memes, or is u want to make some kind of alternative scream button
/// </summary>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Гендер для звука разве что, а для сообщения в чат гендер может переключаться локализацией (просто комментарий, действий не требуется)

Copy link
Author

Choose a reason for hiding this comment

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

Ну, вот я для звука, но может как-то потом

Comment on lines +1 to +5
- files:
- gooool.ogg
license: "CC0-1.0"
copyright: "Modified by SkaldetSkaeg (Github), shortened, cleaned from background sounds, converted to OGG."
source: "https://www.myinstants.com/ru/instant/goool-7073/"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Источник интересный, а у нас есть право лицензировать это как CC0, я уж не говорю о том что использовать?

Copy link
Author

Choose a reason for hiding this comment

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

Я, просто, не уверен, что можно писать “No Known Copyright”.

Насчёт использования, то тут на уровне нашего ТТС.

Copy link
Author

Choose a reason for hiding this comment

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

Предлагаешь свой где-то записать?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Насчёт использования, то тут на уровне нашего ТТС.

Есть 2 существенных отличия от ТТС:

  1. Данный репозиторий не содержит ни методов генерации голоса, ни моделей для генерации, только механизм получения с сервера, что совершенно легально
  2. Сами модели ТТС хоть и могли быть натренерованы на чужих данных, это пока не признаётся однозначным нарушением прав интелликтуальной собственности

Copy link
Collaborator

Choose a reason for hiding this comment

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

А с этим звуком, точнее как ты здесь его определил, есть 2 проблемы:

  1. Мы никак не можем публиковать открытую лицензию на этот файл, но для этого можно указать Custom в поле license
  2. Нет подтверждения тому, что у нас есть лицензия на файл

Copy link
Author

Choose a reason for hiding this comment

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

Короче, снова немного в дебри погрузился. По сути это user generated content, так как постобработка и контекст.

Но что по лицензии хз.

Copy link
Collaborator

Choose a reason for hiding this comment

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

В общем я ещё уточню по этому моменту, но лучше искать звук на который у нас будет лицензия. Можно, я не знаю, попросить Шусса на стриме крикнуть "ГОООЛ" чтоб мы это использовали здесь.

Copy link
Author

Choose a reason for hiding this comment

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

В общем я ещё уточню по этому моменту, но лучше искать звук на который у нас будет лицензия. Можно, я не знаю, попросить Шусса на стриме крикнуть "ГОООЛ" чтоб мы это использовали здесь.

я бы ещё по картинке спросил. Мне её просто пережатую скинули. Лучше попросить наших перерисовать, когда со звуком разберемся?

Copy link
Collaborator

Choose a reason for hiding this comment

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

На самом деле да

Copy link
Author

Choose a reason for hiding this comment

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

с картинкой решено

Comment on lines +1 to +9
- type: dataset
id: goalPhrases
values:
- восклицает
- голосует
- бравирует
- возглашает
- выкрикивает
- вскрикивает
Copy link
Collaborator

Choose a reason for hiding this comment

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

А локализацию можно?

Copy link
Author

Choose a reason for hiding this comment

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

Не совсем понял

Copy link
Collaborator

Choose a reason for hiding this comment

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

сами фразы должны быть в локализации, здесь должны быть только ключи

@stalengd stalengd self-assigned this Nov 4, 2024
@Qwerlink
Copy link

Qwerlink commented Nov 5, 2024

Это ГООООООООООООООООООЛ
image

@github-actions github-actions bot added the Changes: Sprites Изменение затронуло файлы ".rsu / .png" label Nov 5, 2024
Copy link

github-actions bot commented Nov 5, 2024

RSI Diff Bot; head commit 27ba746 merging into 68a91fb
This PR makes changes to 1 or more RSIs. Here is a summary of all changes:

Resources/Textures/SS220/Interface/Actions/actions_meme.rsi

State Old New Status
goal Added

Edit: diff updated after 27ba746

@SkaldetSkaeg SkaldetSkaeg marked this pull request as draft November 6, 2024 02:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changes: C# Изменение затронуло файлы ".cs" Changes: Localization Изменение затронуло файлы ".ftl" Changes: Prototypes Изменение затронуло файлы ".yml" кроме неймспейса "maps" Changes: Sprites Изменение затронуло файлы ".rsu / .png" Need to be discussed Требуется обсуждение перед аппрувом/мёрджем Status: Needs Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants