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

DallasFuncs pokes #346

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

MaddTheSane
Copy link
Contributor

Pull Request Type

  • GitHub Workflow changes
  • Documentation or Wiki changes
  • Build and Dependency changes
  • Runtime changes
    • Render changes
    • Audio changes
    • Input changes
    • Network changes
    • Other changes

Description

This sets all the functions and variables in DallasFuncs.cpp as static, as well as marking them all with [[maybe_unused]]. marking functions and variables as static means that they won't be exported as public symbols by the included source files, and [[maybe_unused]] is because not every plug-in uses all the functions in DallasFuncs.cpp. As DallasFuncs.cpp is included in the scripts files themselves and not build individually, this shouldn't cause any issues.

Checklist

  • I have tested my changes locally and verified that they work as intended.
  • I have documented any new or modified functionality.
  • I have reviewed the changes to ensure they do not introduce any unnecessary complexity or duplicate code.
  • I understand that by submitting this pull request, I am agreeing to license my contributions under the project's license.

Additional Comments

@Arcnor
Copy link
Collaborator

Arcnor commented May 14, 2024

I'm not sure we should touch this file in this way. This is parsed by D3Edit in some unknown way, and it will probably choke on those changes. Granted people can still use the old copy, but things get complicated that way.

Changes here should probably be done in tandem with D3Edit.

@MaddTheSane MaddTheSane mentioned this pull request May 14, 2024
13 tasks
@Lgt2x
Copy link
Collaborator

Lgt2x commented May 14, 2024

I'm not sure we should touch this file in this way. This is parsed by D3Edit in some unknown way, and it will probably choke on those changes. Granted people can still use the old copy, but things get complicated that way.

Changes here should probably be done in tandem with D3Edit.

How hard would it be to get a D3Edit CI build going, that pulls D3 main branch as a submodule?

@Arcnor
Copy link
Collaborator

Arcnor commented May 14, 2024

How hard would it be to get a D3Edit CI build going, that pulls D3 main branch as a submodule?

Should we try to get D3Edit inside the repo first? Not sure what's the final shape we want that to look like, I remember @kevinbentley had some ideas there, but anyway, I haven't compiled D3Edit yet, but I don't think it will be difficult (talkin about the CI part, any fixes to get D3Edit to compile are unknown).

@Lgt2x
Copy link
Collaborator

Lgt2x commented May 14, 2024

How hard would it be to get a D3Edit CI build going, that pulls D3 main branch as a submodule?

Should we try to get D3Edit inside the repo first? Not sure what's the final shape we want that to look like, I remember @kevinbentley had some ideas there, but anyway, I haven't compiled D3Edit yet, but I don't think it will be difficult (talkin about the CI part, any fixes to get D3Edit to compile are unknown).

I haven't compiled D3Edit either, but I'm fine with making it part of this repo

@JeodC
Copy link
Collaborator

JeodC commented May 14, 2024

How hard would it be to get a D3Edit CI build going, that pulls D3 main branch as a submodule?

Should we try to get D3Edit inside the repo first? Not sure what's the final shape we want that to look like, I remember @kevinbentley had some ideas there, but anyway, I haven't compiled D3Edit yet, but I don't think it will be difficult (talkin about the CI part, any fixes to get D3Edit to compile are unknown).

I haven't compiled D3Edit either, but I'm fine with making it part of this repo

+1. I know @jmarshall23 has dev editor to bring in but I don't see why we couldn't have both.

@Lgt2x
Copy link
Collaborator

Lgt2x commented May 22, 2024

I have an almost mature PR to build the internal editor here, hang tight!

@Lgt2x Lgt2x marked this pull request as draft May 22, 2024 18:26
@Lgt2x Lgt2x mentioned this pull request May 29, 2024
9 tasks
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.

None yet

4 participants