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

Traits for roundstart species size #4

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

KillanGenifer
Copy link

@KillanGenifer KillanGenifer commented Sep 11, 2024

Описание PR

Добавляет две новые черты внешности в виде высокого/низкого роста.

Вся смелая часть кода была взята с репы фронтира и слегка отредактировано под нас. Изменён класс Server/Cloning/CloneSystem, чтобы черты оставались после клонирования.

Переделано с нуля. Убрана гибкость в виде настройки роста под каждую расу.

Медиа
image

@Votknyl
Copy link

Votknyl commented Sep 11, 2024

Прикольно, дварфы но люди? эльфы но дварфы?

@Vonsant
Copy link

Vonsant commented Sep 11, 2024

Код под лицензией AGPL v3, соре но соре, его приём будем нарушать лицензию.

@Emechitas
Copy link

Код под лицензией AGPL v3, соре но соре, его приём будем нарушать лицензию.

gosling

@Vonsant
Copy link

Vonsant commented Sep 11, 2024

Код под лицензией AGPL v3, соре но соре, его приём будем нарушать лицензию.

gosling

Нарушение лицензии - нарушение правила 1.2 правил хаба.

lzk228
lzk228 previously requested changes Sep 11, 2024
Content.Server/Cloning/CloningSystem.cs Outdated Show resolved Hide resolved
Content.Server/Cloning/CloningSystem.cs Outdated Show resolved Hide resolved
Resources/Prototypes/Entities/Mobs/Species/base.yml Outdated Show resolved Hide resolved
Resources/Prototypes/Entities/Mobs/Species/dwarf.yml Outdated Show resolved Hide resolved
Resources/Prototypes/Traits/categories.yml Outdated Show resolved Hide resolved
Resources/Prototypes/Traits/sizeattribute.yml Outdated Show resolved Hide resolved
@Vonsant
Copy link

Vonsant commented Sep 11, 2024

@lzk228 Зря стараешься, это чужой код нарушающий лицензию.

@lzk228
Copy link
Collaborator

lzk228 commented Sep 11, 2024

ну не мне же его мёржить, у меня и прав то нет

@KillanGenifer
Copy link
Author

Вроде подправил. Если этот ПР внатуре нарушает правила оффов - перепишу с нуля по другому.

@Morb0
Copy link
Member

Morb0 commented Sep 11, 2024

Вроде подправил. Если этот ПР внатуре нарушает правила оффов - перепишу с нуля по другому.

Переписывай

@MetalSage
Copy link

Код под лицензией AGPL v3, соре но соре, его приём будем нарушать лицензию.

AGPL v3 свободная лицензия, единственное и главное ограничение, весь репозиторий в котором используется код AGPL v3 должен быть лицензирован так же под AGPL v3 и быть полностью открыт

@Vonsant
Copy link

Vonsant commented Sep 11, 2024

Код под лицензией AGPL v3, соре но соре, его приём будем нарушать лицензию.

AGPL v3 свободная лицензия, единственное и главное ограничение, весь репозиторий в котором используется код AGPL v3 должен быть лицензирован так же под AGPL v3 и быть полностью открыт

Да, ты прав. И увы, этот репозиторий под MIT и будет всегда под MIT.

@lzk228
Copy link
Collaborator

lzk228 commented Sep 12, 2024

имхо это очень странно делать это через два булеана в компоненте

@KillanGenifer
Copy link
Author

имхо это очень странно делать это через два булеана в компоненте

Это сделано, чтобы при инициализации компонента выбиралось, в какую сторону изменять рост куклы.

@whateverusername0
Copy link

вывод - просто поменяйте лицензию на AGPLv3 и добавьте MIT как сайд
нахуй париться когда решение на поверхности

@Vonsant
Copy link

Vonsant commented Sep 12, 2024

вывод - просто поменяйте лицензию на AGPLv3 и добавьте MIT как сайд нахуй париться когда решение на поверхности

Уже решено сверху, другой лицензии не будет.

Copy link

@ficcialfaint ficcialfaint left a comment

Choose a reason for hiding this comment

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

Везде должны быть file-scoped namespaces

[RegisterComponent]
public sealed partial class HeightComponent : Component
{
[DataField("tall")]

Choose a reason for hiding this comment

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

Suggested change
[DataField("tall")]
[DataField]

[DataField("tall")]
public bool Tall = false;

[DataField("short")]

Choose a reason for hiding this comment

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

Suggested change
[DataField("short")]
[DataField]

Comment on lines +14 to +15

}

Choose a reason for hiding this comment

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

Suggested change
}
}

physics.SetPositionRadius(uid, id, fixture, circle, circle.Position * scale, circle.Radius * scale, manager);
break;
default:
throw new NotImplementedException();

Choose a reason for hiding this comment

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

Suggested change
throw new NotImplementedException();
break;


private void InitComponent(EntityUid uid, HeightComponent component, ComponentStartup args)
{
if (TryComp<HeightTraitBlacklistComponent>(uid, out var comp))

Choose a reason for hiding this comment

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

Suggested change
if (TryComp<HeightTraitBlacklistComponent>(uid, out var comp))
if (HasComp<HeightTraitBlacklistComponent>(uid))

Comment on lines +36 to +37
var physics = _entityManager.System<SharedPhysicsSystem>();
var appearance = _entityManager.System<AppearanceSystem>();

Choose a reason for hiding this comment

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

Почему это не может быть [Dependency] вместо использования .System()?

Copy link
Author

Choose a reason for hiding this comment

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

Разве это критично важно? Часть кода изменения размера была взята из репы движка, где команда /scale

Choose a reason for hiding this comment

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

ну ты каждый раз про вызове метода Scale получаешь систему вместо того, чтобы один раз инжектнуть Dependency

Copy link
Author

Choose a reason for hiding this comment

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

ну ты каждый раз про вызове метода Scale получаешь систему вместо того, чтобы один раз инжектнуть Dependency

В целом, логично. Тогда отправлю комит с новым кодом на основе всех названных ошибок. Я прост не очень силён в кодинге на шарпе, особенно в связке с таким большим проектом.

Comment on lines +39 to +41
_entityManager.EnsureComponent<ScaleVisualsComponent>(uid);

var appearanceComponent = _entityManager.EnsureComponent<AppearanceComponent>(uid);

Choose a reason for hiding this comment

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

EnsureComp?


appearance.SetData(uid, ScaleVisuals.Scale, oldScale * scale, appearanceComponent);

if (_entityManager.TryGetComponent(uid, out FixturesComponent? manager))

Choose a reason for hiding this comment

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

инвертируй if-блок + TryComp

Comment on lines +33 to +34
}
private void Scale(EntityUid uid, float scale)

Choose a reason for hiding this comment

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

Suggested change
}
private void Scale(EntityUid uid, float scale)
}
private void Scale(EntityUid uid, float scale)

[DataField("short")]
public bool Short = false;

public readonly float TallScale = 1.10f;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Сделай это череp [DataField]

namespace Content.Server.Traits.Assorted
{
[RegisterComponent]
public sealed partial class HeightComponent : Component
Copy link
Collaborator

Choose a reason for hiding this comment

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

Переменная isShort и isTall идея плохая, лучше сделай прототипы роста, или хотя бы enum значение

using System.Numerics;

namespace Content.Server.Traits.Assorted
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Этот блок лишний

if (TryComp<HeightTraitBlacklistComponent>(uid, out var comp))
return;

if (component.Short)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Как говорил выше, реализация отвратительная

public override void Initialize()
{
base.Initialize();
SubscribeLocalEvent<HeightComponent, ComponentStartup>(InitComponent);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Убирать изменение роста при удалении компонента

{
[RegisterComponent]
public sealed partial class HeightTraitBlacklistComponent : Component
{ }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Этот блок лишний просто ставь ;

@@ -0,0 +1,4 @@
- type: traitCategory
id: HeightTraits
name: Рост
Copy link
Collaborator

Choose a reason for hiding this comment

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

используй loc файлы

Copy link
Collaborator

Choose a reason for hiding this comment

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

используй loc файлы

Copy link
Collaborator

Choose a reason for hiding this comment

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

Убери не нужные изменения

@Vonsant
Copy link

Vonsant commented Sep 21, 2024

Лицензию сменили кстати.

@JerryImMouse
Copy link

ебать вы с Торнадо человека обработали 😨

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.